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.

Sign up for GitHub

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

perl: do not remove files that contain build host specific data #157

Draft
wants to merge 1 commit into
base: nilrt/master/scarthgap
Choose a base branch
from
Draft

perl: do not remove files that contain build host specific data #157

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions meta/recipes-devtools/perl/perl-ptest.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ do_install_ptest () {

# Remove a useless timestamp...
sed -i -e '/Autogenerated starting on/d' ${D}${PTEST_PATH}/lib/unicore/mktables.lst

# Remove files with host-specific configuration for building native binaries
rm ${D}${PTEST_PATH}/Makefile.config ${D}${PTEST_PATH}/xconfig.h ${D}${PTEST_PATH}/xconfig.sh
Copy link

Choose a reason for hiding this comment

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

My understanding is that the xconfig.h added in copy step in the commit you mention doesn't have build reproducibility problems. Is the xconfig.h we're adding back here the same that got copied in the other commit or is this the original file that had build reproducibility problems?

Also, is adding back Makefile.config required?

Copy link
Author

Choose a reason for hiding this comment

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

Do we intend to submit this fix upstream?

Yes, this fix is intended to be submitted upstream.

My understanding is that the xconfig.h added in copy step in the commit you mention doesn't have build reproducibility problems. Is the xconfig.h we're adding back here the same that got copied in the other commit or is this the original file that had build reproducibility problems?

One of the comment lines in the copy step says:
"# To make it reproducible let's make it a copy of config.h patch that is specific to the target architecture."
I am not sure what do they mean by "make it reproducible" and whether this new commit really doesn't have build reproducibility problems or is it still the case.

Also, one of the patches in the perl-cross recipe that patches the cross-compile part of the code has this data:
"perl-cross assumes the sources use xconfig.h with -DUSE_CROSS_COMPILE. With perl 5.20.0, it's no longer true. Regardless of what mainline perl uses, let's stick with the old xconfig.h way for now."

This is the reason we wanted to revert the previous changes instead of just copying the config.h data.
@gratian, please correct me if I am wrong here.

Also, is adding back Makefile.config required?

Ideally, we only need xconfig.h file while installing perl ptest tests and the tests doesn't require the other 2 files. But I wanted to just revert the previous changes and didn't want to cause confusion in the upstream, hence kept it as it is.

Copy link

Choose a reason for hiding this comment

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

https://docs.yoctoproject.org/5.0.7/test-manual/reproducible-builds.html has info about reproducible builds.

It also has a section on how to test reproducibility for a recipe.

Since the other commit copied config.h to xconfig.h to fix reproducibility problems, we should do the same here if feasible.

The original commit asserts that Makefile.config causes build reproducibility problems so I think we shouldn't include it back unless either of following is true

  1. Makefile.config doesn't cause build reproducibility problems anymore
  2. Makefile.config is required for something else.

}

python populate_packages:prepend() {
Expand Down