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

Add compatibility for busybox mktemp to sed --in-place check #11269

Merged
merged 1 commit into from Dec 3, 2020

Conversation

qzdanis
Copy link
Contributor

@qzdanis qzdanis commented Dec 2, 2020

Motivation and Context

Busybox's mktemp requires six X's in the template, while the current sed --in-place check invokes mktemp with three. This causes mktemp to fail as the pattern isn't what busybox is expecting.

Description

This PR changes the mktemp template from conftest.XXX to conftest.XXXXXX in config/always-sed.m4. It also extends the templates in config/deb.am and scripts/zfs-tests.sh to contain six X's.

How Has This Been Tested?

Change was tested via regeneration of configure script in a busybox-based docker container and diffing the results against the current master branch to ensure the only change is the mktemp invocation. The resulting script was also run to check if the sed --in-place check passes.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

@ghost ghost added the Status: Code Review Needed Ready for review and testing label Dec 2, 2020
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Thanks! It looks like there are a few other places where this could be a problem. Can you update this PR to make the same change in config/deb.am and scripts/zfs-tests.sh.

Busybox's mktemp requires at least six X's in the template, causing
the current sed --in-place check to fail because the file does not
exist. This change adds additional X's to mktemp templates that do
not already have at least six X's in them.

Signed-off-by: Quentin Zdanis <zdanisq@gmail.com>
@qzdanis
Copy link
Contributor Author

qzdanis commented Dec 2, 2020

Thanks! It looks like there are a few other places where this could be a problem. Can you update this PR to make the same change in config/deb.am and scripts/zfs-tests.sh.

Done! I've also edited the PR description to be more accurate.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 2, 2020
@behlendorf behlendorf merged commit 9109b89 into openzfs:master Dec 3, 2020
@qzdanis qzdanis deleted the fix/busybox-mktemp branch December 3, 2020 19:45
ghost pushed a commit to zfsonfreebsd/ZoF that referenced this pull request Dec 23, 2020
Busybox's mktemp requires at least six X's in the template, causing
the current sed --in-place check to fail because the file does not
exist. This change adds additional X's to mktemp templates that do
not already have at least six X's in them.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Quentin Zdanis <zdanisq@gmail.com>
Closes openzfs#11269
behlendorf pushed a commit that referenced this pull request Dec 23, 2020
Busybox's mktemp requires at least six X's in the template, causing
the current sed --in-place check to fail because the file does not
exist. This change adds additional X's to mktemp templates that do
not already have at least six X's in them.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Quentin Zdanis <zdanisq@gmail.com>
Closes #11269
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Busybox's mktemp requires at least six X's in the template, causing
the current sed --in-place check to fail because the file does not
exist. This change adds additional X's to mktemp templates that do
not already have at least six X's in them.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Quentin Zdanis <zdanisq@gmail.com>
Closes openzfs#11269
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Busybox's mktemp requires at least six X's in the template, causing
the current sed --in-place check to fail because the file does not
exist. This change adds additional X's to mktemp templates that do
not already have at least six X's in them.

Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Quentin Zdanis <zdanisq@gmail.com>
Closes openzfs#11269
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants