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

[Automatic Releases] Skip non user facing weekly releases #338

Open
Tracked by #333
AsherGlick opened this issue Aug 19, 2024 · 8 comments
Open
Tracked by #333

[Automatic Releases] Skip non user facing weekly releases #338

AsherGlick opened this issue Aug 19, 2024 · 8 comments

Comments

@AsherGlick
Copy link
Owner

AsherGlick commented Aug 19, 2024

As brought up in #328 we may have times where we do have new commits, but these commits dont actually change the delivered binaries to users. Cutting a new release when the artifacts in that release are no different from the artifacts in the previous release is undesirable.

Once the weekly build is done, it should compare the build files to the previous weekly's build files. If the files are the same then it skips cutting a new release and instead exits early.

In order to prevent undoing the work of #336, we should somehow indicate that the most recent commit was built, but that there were no changes, and therefore no new tags were added and no new release were created.

A simple solution may be a floating tag that indicates which commit was the most recently built by the release process. Then the workflow checks for that tag instead of just for a release tag when deciding if there are new commits.

@AsherGlick AsherGlick mentioned this issue Aug 19, 2024
8 tasks
@Masgalor
Copy link
Contributor

I have a hard time finding a way to check if the produced binaries are equal to the previous ones or not.

The problem is, that the link binaries burrito_link.exe and d3d11.dll do not build reproducible.
So the checksum for the built artifact differs every time.

If someone could help me get reproducible builds going with gcc, I would be grateful.
But to be honest, maybe it is not worth the hassle.

@AsherGlick
Copy link
Owner Author

AsherGlick commented Aug 19, 2024

I found a way that seems to work for the exe and the dll but I did not check anything else.
The change in #342 prevents a timestamp from being added in the mingw linker, there is probably a way to do this to the exe as well, but I only had to do this to the dll because it had some baked in non stripable data.

strip burrito_link_old.exe
strip burrito_link_new.exe
diff burrito_link_old.exe burrito_link_new.exe
strip d3d11_old.dll
strip d3d11_new.dll
diff d3d11_old.dll d3d11_new.dll

This seems to work. We dont want to strip debug symbols before the release though so the strip call should be done only for the diff, even if it means we have to copy the dll and exe, strip them, and then compare them. Not having debug symbols in the released code sounds optimal, but it makes debugging in the wild much harder and it only small price to pay for greatly improved debugability. Most programs I know of do this.

Hopefully we wont have to deal with this much, but if non-reproducible builds become a problem then we can come up with another solutions. Such as a list of file globs to explicitly ignore, then we hash the input files instead of the output files. It has it's own drawbacks of course, such as if we have different comments inside of a source file or something.

(Technically you dont need to strip the dll files anymore, they actually are fully reproducable now it seems, though I cant say if that will last or not so stripping them anyways is probably a good idea)

@Masgalor
Copy link
Contributor

Thank you for your help, but I get different results with your PR (or I misunderstand something).

Your PR was built twice by the current CI workflow, so I have two artifacts to compare (first run second run)

If I compare the binaries as they are the dll is the same but the exe is not.

diff d3d11_1.dll d3d11_2.dll
diff burrito_link_1.exe burrito_link_2.exe
Binary files burrito_link_1.exe and burrito_link_2.exe differ

If I strip them both are no longer equal.

strip d3d11_1.dll
strip d3d11_2.dll
diff d3d11_1.dll d3d11_2.dll
Binary files d3d11_1.dll and d3d11_2.dll differ

strip burrito_link_1.exe
strip burrito_link_2.exe
diff burrito_link_1.exe burrito_link_2.exe
Binary files burrito_link_1.exe and burrito_link_2.exe differ
strip -V
GNU strip (GNU Binutils; openSUSE Tumbleweed) 2.42.0.20240130-4

diff -v
diff (GNU diffutils) 3.10

@AsherGlick
Copy link
Owner Author

It looks like strip has the flag

-D
--enable-deterministic-archives
   Operate in deterministic mode.  When copying archive members
   and writing the archive index, use zero for UIDs, GIDs,
   timestamps, and use consistent file modes for all files.

   If binutils was configured with
   --enable-deterministic-archives, then this mode is on by
   default.  It can be disabled with the -U option, below.

Maybe SUSE's binutils is not configured with enable-deterministic-archives and ubuntu's is?

$ cat Burrito_Linux_1043/burrito_link/d3d11.dll| sha256sum 
2f5989c38b4eaf4139fcf82fce5fef35becdfd86fa814a9d8c8adaabf9c91dcd  -
$ strip Burrito_Linux_1043/burrito_link/d3d11.dll
$ cat Burrito_Linux_1043/burrito_link/d3d11.dll| sha256sum
1e735630c11f176d8d11610942dc4045fac20300d1bc0c9bdebdeeb9d1bece8f  -

$ cat Burrito_Linux_1044/burrito_link/d3d11.dll| sha256sum
2f5989c38b4eaf4139fcf82fce5fef35becdfd86fa814a9d8c8adaabf9c91dcd  -
$ strip Burrito_Linux_1044/burrito_link/d3d11.dll
$ cat Burrito_Linux_1044/burrito_link/d3d11.dll| sha256sum
1e735630c11f176d8d11610942dc4045fac20300d1bc0c9bdebdeeb9d1bece8f  -

$ cat Burrito_Linux_1043/burrito_link/burrito_link.exe| sha256sum
ce435c1ad7b685bc31ed766546ebb5ccc015691a3fb8aa342f8ef63d81f7b30e  -
$ strip Burrito_Linux_1043/burrito_link/burrito_link.exe
$ cat Burrito_Linux_1043/burrito_link/burrito_link.exe| sha256sum
f1bad3cea11057f68bf06fcf975a8c4204f46895b97a624bc505b2441ad079ef  -

$ cat Burrito_Linux_1044/burrito_link/burrito_link.exe| sha256sum 
870f2e03280af3620b44c5271942f6e09e5daff498daed421641fe53095ae924  -
$ strip Burrito_Linux_1044/burrito_link/burrito_link.exe
$ cat Burrito_Linux_1044/burrito_link/burrito_link.exe| sha256sum
f1bad3cea11057f68bf06fcf975a8c4204f46895b97a624bc505b2441ad079ef  -

$ diff -v
diff (GNU diffutils) 3.8
Copyright (C) 2021 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Paul Eggert, Mike Haertel, David Hayes,
Richard Stallman, and Len Tower.

$ strip -V
GNU strip (GNU Binutils for Ubuntu) 2.38
Copyright (C) 2022 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.

@Technetium1
Copy link

Not having debug symbols in the released code sounds optimal, but it makes debugging in the wild much harder and it only small price to pay for greatly improved debugability

Why would you want to strip debug symbols at this stage? Surely the size isn't too large? Is there some timestamped debug data preventing deterministic build?

Maybe SUSE's binutils is not configured with enable-deterministic-archives and ubuntu's is?

Considering Debian does overrides, this is the most likely case. I've never looked for SUSE sources, so that's something to be further checked.
https://salsa.debian.org/search?search=deterministic&nav_source=navbar&project_id=31349&group_id=4586&search_code=true&repository_ref=binutils-2.40

Related Debian issue about reproducibility and their behavior change for default determinism: https://salsa.debian.org/reproducible-builds/strip-nondeterminism/-/issues/3

Ubuntu seems to be cited as doing the same by default for a while, but I can't immediately find the code to prove it: zephyrproject-rtos/sdk-ng#81 (comment)

@Masgalor
Copy link
Contributor

Indeed SUSE seems to bee the odd one here.
But this does not matter, it is just my local environment, the runner we use to build burrito ubuntu-20.04 behaves like @AsherGlick said. That is all I need to continue with the new workflows, I am almost done with them by the way, I will open the corresponding PRs tomorrow (hopefully).

@AsherGlick
Copy link
Owner Author

@Technetium1

Why would you want to strip debug symbols at this stage? Surely the size isn't too large? Is there some timestamped debug data preventing deterministic build?

We don't want to strip them for a public release, we might want to strip them for diffing though because of things like timestamps and random numbers. I am not totally clear on what or why these exist, only that stripping them yielded better diffing accuracy.

@Masgalor

I am almost done with them by the way, I will open the corresponding PRs tomorrow (hopefully).

That's amazing. Something tells me we want to merge them in slowly so that we can confirm things are working in production the way we want them to work. Also a heads up that we changed a lot of the build workflows in xml_converter #56. Having a pr open like this for so long is not ideal, and had I known some things ahead of time I would have planned it out differently.

@Technetium1
Copy link

@Masgalor maybe this is of interest for your local testing https://github.com/nektos/act

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants