Skip to content

docbook2x: fix strictDeps#358347

Merged
misuzu merged 1 commit intoNixOS:stagingfrom
ofalvai:docbook2x-strictdeps
Dec 15, 2024
Merged

docbook2x: fix strictDeps#358347
misuzu merged 1 commit intoNixOS:stagingfrom
ofalvai:docbook2x-strictdeps

Conversation

@ofalvai
Copy link
Copy Markdown
Contributor

@ofalvai ofalvai commented Nov 23, 2024

Related: #328783

Ref: #178468

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofalvai ofalvai changed the base branch from master to staging November 23, 2024 14:58
@ofborg ofborg bot added 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 Nov 24, 2024
Comment thread pkgs/by-name/do/docbook2x/package.nix Outdated
Copy link
Copy Markdown
Member

@SuperSandro2000 SuperSandro2000 Dec 6, 2024

Choose a reason for hiding this comment

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

Suggested change
buildInputs = [ groff libxml2 opensp libiconv iconv bash ]
buildInputs = [ groff libxml2 opensp libiconv bash ]

also we should set strictDeps

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@ofalvai ofalvai force-pushed the docbook2x-strictdeps branch 2 times, most recently from 4677a38 to e5f4ec6 Compare December 7, 2024 08:49
@ofalvai ofalvai force-pushed the docbook2x-strictdeps branch from e5f4ec6 to 52cdd2a Compare December 7, 2024 08:58
Copy link
Copy Markdown
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Lgtm

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 8, 2024
@ofalvai
Copy link
Copy Markdown
Contributor Author

ofalvai commented Dec 10, 2024

@ofborg eval

@nixos-discourse
Copy link
Copy Markdown

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2143

@misuzu misuzu merged commit 26abce3 into NixOS:staging Dec 15, 2024

nativeBuildInputs = [ makeWrapper perlPackages.perl texinfo libxslt ];
buildInputs = [ groff libxml2 opensp libiconv iconv bash ]
strictDpes = true;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

strictDeps = true;

There is a typo there!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ooops, sorry, and thank you for spotting this. Fix: #400315

Copy link
Copy Markdown
Contributor

@mildsunrise mildsunrise Jun 12, 2025

Choose a reason for hiding this comment

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

this package doesn't work correctly under strictDeps, see #412735. the issue is that configure.ac does a few AC_PATH_PROG for sgml2xml, sx and osx and embeds these paths into the code (so really this should be AC_PATH_TOOL as these are host deps).

this breaks with strictDeps since buildInputs are no longer in PATH. I'm not good at autotools, do you if there's any agreed on way in nixpkgs to package things that make use of AC_PATH_TOOL and friends?

@ofalvai ofalvai mentioned this pull request Apr 20, 2025
13 tasks
mildsunrise added a commit to mildsunrise/nixpkgs that referenced this pull request Jun 12, 2025
Fixes NixOS#412735.

NixOS#358347 / NixOS#400315 left the package broken because configure searces for osx in PATH and hardcodes its path on the Perl code. strictDeps logically prevents the dependency from being in PATH, so an empty string is hardcoded and `sgml2xml-isoent` doesn't work.

The best fix that comes to mind is overriding the autoconf test result via the environment variable.
mildsunrise added a commit to mildsunrise/nixpkgs that referenced this pull request Jun 12, 2025
Fixes NixOS#412735.

NixOS#358347 / NixOS#400315 left the package broken because configure searces for osx in PATH and hardcodes its path on the Perl code. strictDeps logically prevents the dependency from being in PATH, so an empty string is hardcoded and `sgml2xml-isoent` doesn't work.

The best fix that comes to mind is overriding the autoconf test result via the environment variable.
ttuegel pushed a commit to ttuegel/nixpkgs that referenced this pull request Jun 16, 2025
Fixes NixOS#412735.

NixOS#358347 / NixOS#400315 left the package broken because configure searces for osx in PATH and hardcodes its path on the Perl code. strictDeps logically prevents the dependency from being in PATH, so an empty string is hardcoded and `sgml2xml-isoent` doesn't work.

The best fix that comes to mind is overriding the autoconf test result via the environment variable.
wolfgangwalther pushed a commit that referenced this pull request Jun 30, 2025
Fixes #412735.

#358347 / #400315 left the package broken because configure searces for osx in PATH and hardcodes its path on the Perl code. strictDeps logically prevents the dependency from being in PATH, so an empty string is hardcoded and `sgml2xml-isoent` doesn't work.

The best fix that comes to mind is overriding the autoconf test result via the environment variable.

(cherry picked from commit d822ab9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants