Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stable build dates #3828

Merged
merged 3 commits into from
Nov 13, 2020
Merged

Stable build dates #3828

merged 3 commits into from
Nov 13, 2020

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Nov 8, 2020

This PR, along with OpenShot/libopenshot-audio#107 and OpenShot/libopenshot#588, implements an idea I had while discussing Daily Build versions with @rexdk in #3825:

By using the UNIX epoch date of the openshot-qt commit that the build is generated from, instead of the value of time.time(), we can ensure that the set of Daily Build files generated for a given build run all have the same version identifiers in their filename, and differ only by the specifics of the format/architecture targeted.

Before, due to the use of int(time.time()) to set the first part of the build identifier, each of the builders' files was named slightly differently.

Now, instead of e.g. (from the most recent Daily Build run):

OpenShot-v2.5.1-dev2-1604704263-b910d2f6-2cb4eeff-x86_64.AppImage
OpenShot-v2.5.1-dev2-1604704263-b910d2f6-2cb4eeff-x86_64.AppImage.torrent
OpenShot-v2.5.1-dev2-1604704807-b910d2f6-2cb4eeff-x86.exe
OpenShot-v2.5.1-dev2-1604704813-b910d2f6-2cb4eeff-x86_64.exe
OpenShot-v2.5.1-dev2-1604704807-b910d2f6-2cb4eeff-x86.exe.torrent
OpenShot-v2.5.1-dev2-1604704813-b910d2f6-2cb4eeff-x86_64.exe.torrent
OpenShot-v2.5.1-dev2-1604707336-b910d2f6-2cb4eeff-x86_64.dmg
OpenShot-v2.5.1-dev2-1604707336-b910d2f6-2cb4eeff-x86_64.dmg.torrent

We'd instead have (based on the timestamp of the triggering commit):

OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86_64.AppImage
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86_64.AppImage.torrent
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86.exe
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86_64.exe
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86.exe.torrent
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86_64.exe.torrent
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86_64.dmg
OpenShot-v2.5.1-dev2-1604699062-b910d2f6-2cb4eeff-x86_64.dmg.torrent

Much nicer, IMHO.

This PR adds capture of the openshot-qt commit timestamp (as a UNIX epoch time), and storage in the share/openshot-qt metadata file.

It also updates build-server.py to:

  1. Expect a new field COMMIT_DATE_UNIX, when reading the metadata files for all three architectures
  2. Prefer int(version_info['openshot-qt']['COMMIT_DATE_UNIX']) as the source of the timestamp component of the build filename, although it will fall back to int(time.time()) if for some reason the commit date isn't available.

This change may conceivably cause the Daily Build timestamps to initially go backwards slightly in time, if the most recent commit ends up being timestamped before the wall-clock timestamps of the previous set of builds. But even if that happens, it should hopefully be a one-time thing.

@ferdnyc ferdnyc added 📅 Daily Build This is an issue involving a daily build. 📦 packaging An issue or PR related to the official package builds labels Nov 8, 2020
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 8, 2020

Aw... crap. I used the author commit timestamp (git log --date=unix --format="%ad")... but I should've used the committer timestamp (git log --date=unix --format="%cd"). That way, the timestamps are guaranteed to be in numerical order, and e.g. builds from two different branches can't end up with the same timestamp. (Well, except for the highly unlikely coincidence of them having been committed at the exact same second).

...Now I have to go edit all those files! 😭

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 8, 2020

Aw... crap. I used the author commit timestamp (git log --date=unix --format="%ad")... but I should've used the committer timestamp (git log --date=unix --format="%cd").

Huh. Surprisingly enough, now that I've changed it everywhere, this doesn't actually seem to make a difference. At least locally on my system, %ad and %cd always seem to return the same timestamp, though I can't fathom how that's possible for things like merged branches.

¯\_(ツ)_/¯ Oh well. %cd is "more correct" even if it doesn't actually matter in practical terms.

@jonoomph
Copy link
Member

@ferdnyc Also, just FYI, I think the reason we based each daily build on the current time was to prevent collisions when uploading Artifacts to our Daily Build release on GitHub. I seem to recall lots of failures when trying to upload the same daily build again. So, fair warning. 👍

@jonoomph
Copy link
Member

jonoomph commented Nov 12, 2020

I also think those dates (i.e. commit date) is an ENV variable made available on GitLab for each build. But that might not be any better, since it's only available on the current branch/project.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 12, 2020

@jonoomph

@ferdnyc Also, just FYI, I think the reason we based each daily build on the current time was to prevent collisions when uploading Artifacts to our Daily Build release on GitHub. I seem to recall lots of failures when trying to upload the same daily build again. So, fair warning.

Well, in theory there shouldn't really be collisions — at least, I'm hoping not. In theory no two sets of build jobs should be generated with the same "fingerprint", since any new build will be triggered by some commit that changes one of the name components. I suppose if a successful job is manually restarted for the exact same set of commits, even after the first successful one has uploaded artifacts, then there might be an issue.

Or... oh, hm. There is one situation I can think of where that might happen, and it's one I didn't account for. If a previous job partly succeeded, but had to be re-started, the re-builds on the jobs that succeeded the previous time might fail to upload, because the previous run already uploaded the same output file. Hmm.

I wonder if there's a way to make "file already exists" a non-fatal condition, since in those cases it's actually fine for the job to succeed without uploading; it doesn't need to upload since there was already a successful build uploaded.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 12, 2020

@jonoomph

I wouldn't actually mind the current timestamp, as long as all the files built from the same run used the same current timestamp, so that it was easier to determine which files "go together" and are built with the same contents.

Maybe the right approach is to just have a script job in the pipeline, in between the libopenshot build and the multiple openshot-qt architecture jobs, that does nothing but generate the base filename for all of the build artifacts in that run, and passes it on to all of the individual architecture builders?

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

@jonoomph

The more I think about this, the more I think my last comment is the way to go, instead of the changes in this PR. What we need is a "basename generation" pipeline step that runs at the beginning of the openshot-qt CI pipeline. That way, all of the builds in any given pipeline will use the same filenames (with different extensions/suffixes), but that base filename can be generated using the current time so every pipeline is guaranteed to produce files that are named differently from any of the others.

I'm just not 100% sure whether that can be done by just editing the .gitlab-ci.yml file, or if there's configuration that needs to be done administratively in the Gitlab instance to modify the pipeline steps. Any pointers?

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

Hmmm. Looking at the docs, another option would be to use the CI_PIPELINE_ID instead of the current time. That should still be unique per run, but the same for all builders in the run. It's also waay shorter (four digits, currently, eventually 5), so that's nice too.

@jonoomph
Copy link
Member

@ferdnyc Yes, I like the idea of tacking the CI_PIPELINE_ID onto the filename! Like you say, much shorter, and less emphasis than a full datetime epoch.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

I shall update the PR!

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

Done, and I closed the two in the other repos. There's really no need to capture CI_PIPELINE_ID in the metadata files for the libraries.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

🙄 Random Travis failure is random. ("Couldn't get key" for the libopenshot-daily PPA, on some of the build jobs.)

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

Merging this. The project Mac builder still isn't finished because it's waiting for Notarization, but all of the builds were successful.

@ferdnyc ferdnyc merged commit 582c220 into develop Nov 13, 2020
@ferdnyc ferdnyc deleted the stable-build-dates branch November 13, 2020 22:42
@ferdnyc
Copy link
Contributor Author

ferdnyc commented Nov 13, 2020

Me:

Or... oh, hm.

Hmm.

Hmmm.

..Not intentional, but subconsciously my pensiveness is surprisingly linear in its progression.

I guess the only thing to say regarding that is: "Hmmmm."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📅 Daily Build This is an issue involving a daily build. 📦 packaging An issue or PR related to the official package builds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants