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

Make the generated pkgconfig files reproducible. #3211

Merged
merged 1 commit into from Mar 25, 2018

Conversation

lamby
Copy link
Contributor

@lamby lamby commented Mar 10, 2018

Whilst working on the Reproducible Builds effort [0], we noticed
that meson creates non-reproducible pkgconfig files as it iterates
over Python sets.

This was originally filed in Debian as #892515 [1].

[0] https://reproducible-builds.org/
[1] https://bugs.debian.org/892515

@sarum9in
Copy link
Contributor

It looks like there are SharedLibrary objects inside pub_libs and you can't sort them together with strs since < is not defined for this.

@lamby
Copy link
Contributor Author

lamby commented Mar 10, 2018

@sarum9in Thanks for the heads-up; indeed, the pub_libs can contain str or SharedLibrary.. trying a different way :)

@lamby
Copy link
Contributor Author

lamby commented Mar 10, 2018

@sarum9in There you go :)

@jpakkane
Copy link
Member

Yes, changing -lfoo -lbar into -lbar -lfoo can break linking and is not really something we can do, we have to preserve the order specified by the user. The fact that we have not done that is a bug. Maybe we could use OrderedSet instead?

@sarum9in
Copy link
Contributor

I didn't find OrderedSet but I don't think it would work since normally OrderedSet is just a sorted collection and this is not what we want. I agree with @xclaesse that we should change dups, I think we could just go for O(n^2) exclusion algorithm over list. The question is how to decide which occurrence to keep?

@xclaesse
Copy link
Member

@lamby I'm not sure, but IIRC when doing static link when libfoo uses symbols from libbar and you do "-lfoo -lbar" when it will add libfoo it won't have symbols from libbar yet and it will fail to link.

@xclaesse
Copy link
Member

@sarum9in I would suggest (but I'm not sure at all!) to just keep the first. At least that would be reproductible and probably what you need 99% of the time. I think there are corner cases where it could still fail, but in that case I would say just don't use the pkgconfig.generate() and do your pc file by hand.

Does that make sense?

@sarum9in
Copy link
Contributor

@xclaesse I am slightly more concerned about a case when we use some parameters like -Wl,--whole-archive -lfoo -Wl,--no-whole-archive. However it is already broken so... Might be fine.

@lamby
Copy link
Contributor Author

lamby commented Mar 11, 2018

Thank you all for the quick feedback. I've pushed a another revision here, just seeing if all the tests pass..

@@ -167,12 +174,12 @@ def generate_pkgconfig_file(self, state, deps, subdirs, name, description,
ofile.write('URL: %s\n' % url)
ofile.write('Version: %s\n' % version)
if len(deps.pub_reqs) > 0:
ofile.write('Requires: {}\n'.format(' '.join(deps.pub_reqs)))
ofile.write('Requires: {}\n'.format(' '.join(sorted(deps.pub_reqs))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of doing _fn() if you still call sorted() here? I think the idea to avoid set()/sorted() was to keep original order and remove duplicates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was an error to keep this sorted.

Whilst working on the Reproducible Builds effort [0], we noticed
that meson creates non-reproducible pkgconfig files as it relies
on Python set ordering.

This was originally filed in Debian as #892515 [1].

 [0] https://reproducible-builds.org/
 [1] https://bugs.debian.org/892515
@lamby
Copy link
Contributor Author

lamby commented Mar 11, 2018

Updated :)

@jpakkane
Copy link
Member

We currently have a more urgent pkg-config emergency on our hands so unfortunately this will have to wait until that has been solved.

@lamby
Copy link
Contributor Author

lamby commented Mar 16, 2018

a more urgent pkg-config emergency on our hands

Oh? :)

@jpakkane
Copy link
Member

@jpakkane
Copy link
Member

I'll merge this as it fixes the issue and does not change behaviour. However as discussed above the dedup logic itself is probably broken for static linking and other stuff. We may need to change that to work properly. However that is an issue for another PR. Thanks.

@jpakkane jpakkane merged commit 979eaa8 into mesonbuild:master Mar 25, 2018
@lamby
Copy link
Contributor Author

lamby commented Mar 25, 2018

Thanks!

@lamby lamby deleted the reproducible-pkgconfig branch March 25, 2018 16:07
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

Successfully merging this pull request may close these issues.

None yet

4 participants