Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Jun 22, 2022

Description of changes

Closes #178625

The busybox version of mktemp requires exactly six X characters in the argument to mktemp, unlike the coreutils version of mktemp.

Let's accomodate packages, like epson-escpr2, which fool setup.sh into using the busybox version instead of the stdenv version.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Closes #178625

The `busybox` version of `mktemp` requires exactly six `X` characters
in the argument to `mktemp`, unlike the `coreutils` version of `mktemp`.

Let's accomodate packages, like `epson-escpr2`, which fool `setup.sh`
into using the `busybox` version instead of the `stdenv` version.
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jun 22, 2022
@Mindavi
Copy link
Contributor

Mindavi commented Jun 23, 2022

https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/mktemp.html

The template should have at least six trailing 'X' characters. These characters are replaced with characters from the portable filename character set in order to generate a unique name.

Looks good to me. I like that stdenv is made to be very portable, that will make porting easier if some other shell is used in the future.

@Mindavi Mindavi merged commit adafa1c into NixOS:staging Jun 23, 2022
@ghost
Copy link
Author

ghost commented Jun 23, 2022

https://refspecs.linuxbase.org/LSB_3.1.0/LSB-Core-generic/LSB-Core-generic/mktemp.html

The template should have at least six trailing 'X' characters.

One of the great parts about contributing to nixpkgs is learning unexpected stuff like this :)

I like that stdenv is made to be very portable, that will make porting easier if some other shell is used in the future.

Well, technically coreutils is part of stdenv... in fact that was the main reason why I did not test my changes on any other shell or "utils".

I think we ought to alert people to the fact that setup.sh currently works with busybox, and there are packages which depend on this behavior.

One way to do this is to make sure that applications/misc/hello/default.nix fails to build if people don't preserve this behavior. For example, hello deliberately passes a function (rather than an attrset) to mkDerivation so people are forced to make sure they don't break that capability. Unfortunately making hello use busybox to execute setup.sh would add an unnecessary and large dependency to hello, so that isn't a good idea.

Any ideas on how else to make sure people notice if they make changes to setup.sh that don't work with busybox?

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

Labels

6.topic: stdenv Standard environment 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants