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

Sort list of input files #21553

Merged
merged 2 commits into from Aug 29, 2018
Merged

Sort list of input files #21553

merged 2 commits into from Aug 29, 2018

Conversation

bmwiedemann
Copy link
Contributor

While working on reproducible builds for openSUSE, I found that
the godot package differed for every build.
These patches fix most issues by sorting lists of input files.

There is also a correctness fix included. Without that, sorting failed, because str < list is not a valid python operation.

See individual commit messages for details.

@bmwiedemann
Copy link
Contributor Author

Fun fact: while analyzing these issues and creating minimal, well-defined patches I spent several hours "waiting for godot" compilation to finish ;-)

@akien-mga
Copy link
Member

Seems fine, though in Godot we prefer such strongly-related commits to be grouped together as one. Could you squash them?

There are some other occurrences of glob.glob, do they also need sorting?

$ rg glob.glob\\\(
SConstruct
26:for x in glob.glob("platform/*"):
546:            file_stat = [(x, os.stat(x)[6:8]) for x in glob.glob(os.path.join(self.path, '*', '*'))]

methods.py
16:        filetype = glob.glob(dir_path + "/" + filetype)
104:    files = glob.glob("modules/*")

editor/SCsub
73:    tlist = glob.glob(path + "/translations/*.po")
78:    flist = glob.glob(path + "/../thirdparty/fonts/*.ttf")
79:    flist.append(glob.glob(path + "/../thirdparty/fonts/*.otf"))

scene/resources/default_theme/make_header.py
18:pixmaps = glob.glob("*.png")
43:shaders = glob.glob("*.gsl")

@akien-mga
Copy link
Member

scene/resources/default_theme/make_header.py
18:pixmaps = glob.glob("*.png")
43:shaders = glob.glob("*.gsl")

This one you can probably leave as is, it's not used directly in the buildsystem (and seems partly bogus, shaders should be .glsl, not .gsl).

@bmwiedemann
Copy link
Contributor Author

I tested that only these globs needed sorting for our package to be able to produce bit-identical results.
Squashed the 3 sort patches into 1 now and left the bugfix separate, because it is of a different nature.

We want to add the individual strings to the list
and not add a list object to the list.

Without this patch, sorting failed because "str < list"
is not a valid operation in python.
so that godot package builds reproducibly
in spite of indeterministic filesystem readdir order
and http://bugs.python.org/issue30461

See https://reproducible-builds.org/ for why this is good.

Sort font input file list, so that builtin_fonts.gen.h
is created in a reproducible way

Sort list of platforms, so that editor/register_exporters.gen.cpp
is created in a reproducible way

Sort list of source files, so that .a files and resulting godot binaries
are created in a reproducible way
@akien-mga
Copy link
Member

Perfect, thanks!

@bmwiedemann
Copy link
Contributor Author

Please consider backporting. At the very least for the bugfix commit, but the sorting does not hurt either.

@akien-mga akien-mga merged commit 4af5c2a into godotengine:master Aug 29, 2018
@akien-mga
Copy link
Member

Cherrypicked for 3.0.7. (Only commit f312582, the second one doesn't apply.)

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

Successfully merging this pull request may close these issues.

None yet

2 participants