-
Notifications
You must be signed in to change notification settings - Fork 24
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
perl: do not remove files that contain build host specific data #157
base: nilrt/master/scarthgap
Are you sure you want to change the base?
perl: do not remove files that contain build host specific data #157
Conversation
The following perl ptests: - dist/ExtUtils-ParseXS/t/001-basic - dist/ExtUtils-ParseXS/t/002-more - dist/ExtUtils-ParseXS/t/003-usage - cpan/ExtUtils-Constant/t/Constant - cpan/ExtUtils-MakeMaker/t/02-xsdynamic are erroring out with: | /usr/lib/perl/ptest/perl_langinfo.h:8:10: fatal error: xconfig.h: No such file or directory | 8 | #include "xconfig.h" xconfig.h contains references to the build host architecture and was removed by commit 2e0f30c ("perl: do not install files that contain build host specific data") However, it is still included from various other places including these tests, and we are still depending on build host architecture data by including the patches from perl-cross recipe, a dependency to perl recipe. xconfig.h was added back as a copy step in the commit f90922c ("update 5.36.1 -> 5.38.0") but was not added back in perl-ptest include file. This new commit removes the step that previously removed build host specifc data from PTEST_PATH. The changes in this commit fixes the test failures. Signed-off-by: Rajendra Desai <rajendra.desai@emerson.com>
There was a problem hiding this comment.
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?
@@ -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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
- Makefile.config doesn't cause build reproducibility problems anymore
- Makefile.config is required for something else.
The following perl ptests:
are erroring out with:
| /usr/lib/perl/ptest/perl_langinfo.h:8:10: fatal error: xconfig.h: No such file or directory
| 8 | #include "xconfig.h"
xconfig.h contains references to the build host architecture and was removed by commit 2e0f30c ("perl: do not install files that contain build host specific data")
However, it is still included from various other places including these tests, and we are still depending on build host architecture data by including the patches from perl-cross recipe, a dependency to perl recipe.
xconfig.h was added back as a copy step in the commit f90922c ("update 5.36.1 -> 5.38.0") but was not added back in perl-ptest include file. This new commit removes the step that previously removed build host specifc data from PTEST_PATH. The changes in this commit fixes the test failures.
(AB#2990669 for reference)
Created this PR as I wanted to get the changes reviewed by the team before I could submit a patch upstream.