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

1.7.0: lv2 and vst3 shared libraries do not succeed in Arch reproducible build tooling checks #2389

Closed
dvzrv opened this issue Jul 29, 2020 · 17 comments
Labels
Awaiting User Information Self-explanatory Feature Request New feature request Infrastructure Issues related to repository, CI/CD, installers, etc. Linux Issues which only occur on Linux

Comments

@dvzrv
Copy link

dvzrv commented Jul 29, 2020

Describe the bug
Hi! When building a package for 1.7.0 on Arch Linux I ran our reproducible builds tooling against the resulting package.

Unfortunately the built shared libraries for the lv2 and vst3 plugins are not reproducible.
The diffoscope log is very big and I couldn't attach it to this ticket. I've uploaded it here.

Please let us know your surge version
This information is all in the menu/about screen

  • Surge Version: 1.7.0
  • Plugin type: lv2, vst3
  • Bits: 64

To Reproduce
This is roughly what happens when building the package:

  export LDFLAGS="${LDFLAGS},-z,noexecstack"
  cmake -DCMAKE_INSTALL_PREFIX='/usr' \
        -DCMAKE_BUILD_TYPE='None' \
        -W no-dev \
        -B build \
        -S .
  make VERBOSE=1 -C build
  # install lv2 and vst3 plugins
  install -vDm 755 "build/surge_products/${_name}.lv2/${_name}.so" \
    -t "${pkgdir}/usr/lib/lv2/${_name}.lv2/"
  install -vDm 644 "build/surge_products/${_name}.lv2/"*.ttl \
    -t "${pkgdir}/usr/lib/lv2/${_name}.lv2/"
  install -vDm 755 "build/surge_products/${_name}.vst3/Contents/${CARCH}-linux/${_name}.so" \
    -t "${pkgdir}/usr/lib/vst3/${_name}.vst3/Contents/${CARCH}-linux/"
  install -vdm 755 "${pkgdir}/usr/share/${pkgname}"
  # install resources
  cp -av resources/data/* "${pkgdir}/usr/share/${pkgname}"
  install -vDm 644 {AUTHORS,README.md} -t "${pkgdir}/usr/share/doc/${pkgname}"

Afterwards I try to reproduce the resulting package:

repro -ndf surge-1.7.0-1-x86_64.pkg.tar.zst

The tool will build a reproducer package and compare it to the initial package using diffoscope (link to log above).

Expected behavior

Running repro against the package returns a reproducer package for which diffoscope returns an empty log (package is reproducible).

Screenshots
n/a

Desktop (please complete the following information):

  • OS: Arch Linux
  • Host n/a
  • Version n/a

Additional context

It seems that surge is not bit reproducible (the binary content differs after build).

  • gcc: 10.1.0
  • cmake: 3.18.0
@dvzrv dvzrv added the Bug Report Item submitted using the Bug Report template label Jul 29, 2020
@baconpaul baconpaul changed the title 1.7.0: lv2 and vst3 shared libraries are not reproducible 1.7.0: lv2 and vst3 shared libraries do not succeed in Arch reproducible build tooling checks Jul 29, 2020
@baconpaul
Copy link
Collaborator

Well first things first: We put build time, git hash, and build host into our binary. This is amazingly useful in our testing and bug reporting. Seems that's gonna break your check.

But it looks like a lot of work to make surge meet the constraints in that website!

@baconpaul baconpaul added this to the Currently Unscheduled milestone Jul 29, 2020
@baconpaul
Copy link
Collaborator

wow so much of that output seems to be errors thrown from std::map destructors and stuff. Is there something about reproducibility and STL which is an easy first step? Why would those even be errors?

@dvzrv
Copy link
Author

dvzrv commented Jul 29, 2020

We put build time, git hash, and build host into our binary

Ouf, yes, that would explain a lot :D
Is there a way I can remove this? Any pointers appreciated!
Especially the timestamp and build host are not relevant and will change (and thus break reproducibility). This would really be where reproducible builds comes to the rescue! :)

Is there something about reproducibility and STL which is an easy first step?

This I can't answer you, but will try to find people and attach them to this ticket who will! :)

@baconpaul
Copy link
Collaborator

Is there a way I can remove this? Any pointers appreciated!

src/common/version.cpp.in replace all the @SURGE_BLAH@ with either a blank or a 0.

I would really like the version and git hash to stay but you are right the timestamp and build host we could skip if needs be. You could do that with a variety of ways

@dvzrv
Copy link
Author

dvzrv commented Jul 29, 2020

Okay, thanks! The version and git hash can stay I guess, as they should be immutable in a source tarball.

@baconpaul
Copy link
Collaborator

yup!

@dvzrv
Copy link
Author

dvzrv commented Jul 29, 2020

Hm, replacing SURGE_BUILD_{DATE,TIME,ARCH,FQDN} only marginally reduced the output of diffoscope (see here).

@dvzrv
Copy link
Author

dvzrv commented Jul 29, 2020

AFAICS the major part of the inconsistency is ordering/sorting of the compiled in/referenced SVG data.
Maybe something can be done about this? Is this auto-generated?

@baconpaul
Copy link
Collaborator

It is autogenerated. There's a python script which generates it. It would be super easy to sort in that script.

The script is scripts/linux/emit-vector-piggy.py and yeah it just does an os.listdir without a sort.

Can you take a swing at hacking that into two parts (do the os.listdir into an array then sort it?) Are you seeing different orders in build/lintemp/ScalablePiggy.S between builds?

@dvzrv
Copy link
Author

dvzrv commented Jul 29, 2020

Ah nice! I'll look into it tomorrow. Time to zzzz

Are you seeing different orders in build/lintemp/ScalablePiggy.S between builds?

No, but the final binaries have sorting issues with svg data. The diffoscope output is only about the final package data

@baconpaul
Copy link
Collaborator

Right; so the svgs layout should be reflected by the set of includes in ScalablePiggy.S. If that list isn't sorted then indeed you will get build-to-build changes. But the overall size should be the same in any case so it would be a local change but it wouldn't push the rest of your dll around. I wonder if the link order is getting changed somehow between builds?

Cmake should be deterministic but there are a couple of places where we use GLOB in the CMake (I know, I know) on linux. So the other thing you may want to try (using the theory that the glob gives non-reproducible ordering also) is to replace the LINUX_VSTGUI_GLOB and SURGE_VST3_GLOB with a hand typed list of those files. Or at least, after the glob, do a sort (that is, do a list( SORT ${SURGE_VST3_GLOB} ) and same for VSTGUI_GLOB where those globs occur.

@dvzrv
Copy link
Author

dvzrv commented Jul 31, 2020

Hm, seems it'll take me until next week as I'm off the weekend. I'll definitely look into that.
I have tried to ping you on IRC about that Python script: Would it be okay to make this only compatible to Python >3.6? Anything else doesn't make much sense IMHO (especially not compatibility to Python2).
Potentially I could even throw some testing/linting on that.

@baconpaul
Copy link
Collaborator

Python3 is fine. The script is pretty Straightforward.

3.6 vs earlier 3 unsure. We build back to ubuntu 16 so that may be 3.5?

I’m not really on irc; the surge irc bridged to our slack but I guess that is not running anymore. GitHub or slack best.

Enjoy your weekend!

@baconpaul baconpaul added Linux Issues which only occur on Linux Awaiting User Information Self-explanatory Feature Request New feature request Infrastructure Issues related to repository, CI/CD, installers, etc. and removed Bug Report Item submitted using the Bug Report template labels Nov 29, 2020
@mkruselj
Copy link
Collaborator

@baconpaul Is this still an issue? Good to close?

@baconpaul
Copy link
Collaborator

It is still true but I certainly don’t plan on working on it ever! And think it would be very very hard to achieve. So I suggest we close it unless someone from the arch community decides to make it a priority and wants to invest in fixing it.

@mkruselj mkruselj removed this from the Currently Unscheduled milestone Aug 26, 2021
@cbix
Copy link
Contributor

cbix commented May 23, 2022

I didn't check with the typical repro tooling, but successive builds on the same build environment yield the same checksum if SURGE_BUILD_{DATE,TIME,HASH} are replaced static values. An easy first step would be in the cmake definitions to generate the date and time values from the SOURCE_DATE_EPOCH environment variable, which is used by repro tooling and contains a unix timestamp (can be 0).

I'm not that good with CMake but in bash I'd do

SURGE_BUILD_DATE=$(date +"%Y-%m-%d" -d @${SOURCE_DATE_EPOCH:-0})
SURGE_BUILD_TIME=$(date +"%H:%M:%S" -d @${SOURCE_DATE_EPOCH:-0})

the build hash could then maybe be generated from these variables so it doesn't change for a fixed SOURCE_DATE_EPOCH.

@baconpaul
Copy link
Collaborator

Oh neat. Still happy to merge a change which does what you need and is tested as long as it passes our ci!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting User Information Self-explanatory Feature Request New feature request Infrastructure Issues related to repository, CI/CD, installers, etc. Linux Issues which only occur on Linux
Projects
None yet
Development

No branches or pull requests

4 participants