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

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

Conversation

rajendra-desai-ni
Copy link

@rajendra-desai-ni rajendra-desai-ni commented Mar 3, 2025

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.


(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.

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>
@rajendra-desai-ni rajendra-desai-ni requested review from gratian and a team March 3, 2025 09:35
Copy link

@chaitu236 chaitu236 left a 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

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.

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.

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.

2 participants