Skip to content

autoconf-archive: Fix build on FreeBSD#311760

Closed
rhelmot wants to merge 1 commit intoNixOS:masterfrom
rhelmot:freebsd-minimal3/autoconf-archive
Closed

autoconf-archive: Fix build on FreeBSD#311760
rhelmot wants to merge 1 commit intoNixOS:masterfrom
rhelmot:freebsd-minimal3/autoconf-archive

Conversation

@rhelmot
Copy link
Contributor

@rhelmot rhelmot commented May 14, 2024

Description of changes

part of #296581

texinfo is required for building docs on FreeBSD native.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
    • x86_64-freebsd
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 14, 2024
@Ericson2314 Ericson2314 force-pushed the freebsd-minimal3/autoconf-archive branch from 177f193 to 9dfb8ba Compare May 15, 2024 04:52
@Ericson2314 Ericson2314 changed the base branch from staging to master May 15, 2024 04:52
@rhelmot rhelmot added the 6.topic: bsd Running or building packages on BSD label May 15, 2024
@Ericson2314
Copy link
Member

  1. I think we should target master where possible. We can merge master into staging at any time (last I checked).

  2. What exactly is the reason for this? build-time conditional deps, or cross vs native divergence, feels rather peculiar to me.

@rhelmot
Copy link
Contributor Author

rhelmot commented May 15, 2024

       last 25 log lines:                                                                                                          
       > cd "$am__cwd"; \                                                                                                          
       > if /nix/store/5fxljnxqia0f76xfykzqrr6p60yrkv1c-bash/bin/bash '/build/autoconf-archive-2023.02.20/build-aux/missing' makeinfo   -I . \
       >  -o autoconf-archive.info autoconf-archive.texi; \                                                                        
       > then \                                                                                                                    
       >   rc=0; \                                                                                                                 
       >   CDPATH="${ZSH_VERSION+.}:" && cd .; \                                                                                   
       > else \                                                                                                                    
       >   rc=$?; \                                                                                                                
       >   CDPATH="${ZSH_VERSION+.}:" && cd . && \                                                                                 
       >   $restore $backupdir/* `echo "./autoconf-archive.info" | sed 's|[^/]*$||'`; \                          
       > fi; \                                                                                                                     
       > rm -rf $backupdir; exit $rc                                                                                               
       > /build/autoconf-archive-2023.02.20/build-aux/missing: line 81: makeinfo: command not found              
       > WARNING: 'makeinfo' is missing on your system.                                                                            
       >          You should only need it if you modified a '.texi' file, or                                  
       >          any other file indirectly affecting the aspect of the manual.                                     
       >          You might want to install the Texinfo package:                                                                                                                                                                                                      
       >          <https://www.gnu.org/software/texinfo/>
       >          The spurious makeinfo call might also be the consequence of
       >          using a buggy 'make' (AIX, DU, IRIX), in which case you might
       >          want to install GNU make:
       >          <https://www.gnu.org/software/make/>
       > make[1]: *** [Makefile:325: autoconf-archive.info] Error 127
       > make[1]: Leaving directory '/build/autoconf-archive-2023.02.20/doc'
       > make: *** [Makefile:420: install-recursive] Error 1
       For full logs, run 'nix-store -l /nix/store/3ypq5y2pdmdwh3m0rqjgmf6jivm8hrxm-autoconf-archive-2023.02.20.drv'.

I believe this is in turn because of https://github.com/NixOS/nixpkgs/pull/311795/files#diff-4b5da784f51cdf538452c429273232dce5d0155b75dd461ab67c0d7665c07322. We have to change uname unqualified to /nix/store/.../bin/uname, which triggers the rebuild.

@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels May 15, 2024
@alyssais alyssais self-requested a review May 15, 2024 08:08
Copy link
Member

Choose a reason for hiding this comment

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

Why just on FreeBSD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See conversation with John on the main thread. The unqualified-uname problem which precipitates this is present for a few operating systems, but I believe FreeBSD is the only one we support building from. We can enable this for all platforms and it will be harmless, but I think this signals the mechanism more clearly. I can also add a comment.

Copy link
Member

Choose a reason for hiding this comment

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

what does texinfo have to do with uname?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Patching the unqualified uname trips some modified-file check which causes some additional configure time logic which invokes texinfo. See above for the exact error message which explains this.

Copy link
Member

Choose a reason for hiding this comment

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

Could maybe use a comment, then, since not everybody's going to have a FreeBSD build machine around to check for themselves why it's required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it finally clicked what the "modified-file check" means / how that matters. Got it.

Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Looks good with a squash.

@Ericson2314
Copy link
Member

Sorry one more question, how come the /usr/bin/uname isn't a problem normally? Does it depend on whether we need to regenerate configure?

Seems like this should be tied to autoreconfHook somehow...

@rhelmot
Copy link
Contributor Author

rhelmot commented May 15, 2024

horrifyingly enough, in the autoconf code which is THIS old, some of the invocations of uname for some operating systems look it up via PATH and some hardcode /usr/bin/uname.

edit: not actually sure anymore if the oldness has anything to do with it. I have vague memory of this mattering for many other packages.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. labels May 15, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label May 16, 2024
@rhelmot rhelmot force-pushed the freebsd-minimal3/autoconf-archive branch from 72ce672 to 6c83371 Compare May 17, 2024 22:13
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one person. label May 18, 2024
@rhelmot
Copy link
Contributor Author

rhelmot commented May 21, 2024

Okay I investigated the consequences of the uname patch some more.

It looks like autoreconfHook DOES solve the same problem as the uname patch hook in stdenv that necessitates the texinfo package being added here, on account of this very recent autoconf commit. I set up a branch that removes the stdenv hook and instead just adds autoreconfHook to everything that will take it, and just does the patch inline for everything else. Unfortunately, due to the nature of stdenv, this was 36 packages changed, of which 17 of them were unable to use autoreconfHook. I think we would just be better just doing it the way I was doing it initially, with the stdenv hook. I can add a comment with the nature of this dependency.

@rhelmot rhelmot force-pushed the freebsd-minimal3/autoconf-archive branch from 6c83371 to 170b5e9 Compare May 21, 2024 07:37
@alyssais
Copy link
Member

Using a hook sounds okay, but it shouldn't be a default stdenv hook, because that'd be next to impossible to remove later. We can add it as an input to the 17 packages that need it, and then phase it out as their configure scripts are updated.

@Ericson2314
Copy link
Member

What about updateAutotoolsGnuConfigScriptsHook? It always has good support in the existing native stdenvs too. (I think I was confusing autoreconfHook and that.)

@rhelmot
Copy link
Contributor Author

rhelmot commented May 22, 2024

Looks like since the linked PR this is no longer needed :)

@rhelmot rhelmot closed this May 22, 2024
@rhelmot rhelmot deleted the freebsd-minimal3/autoconf-archive branch May 22, 2024 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: bsd Running or building packages on BSD 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants