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

stdenv/cross: remove now-redundant file nativeBuildInput on mingw #176597

Merged
merged 1 commit into from Nov 21, 2022
Merged

stdenv/cross: remove now-redundant file nativeBuildInput on mingw #176597

merged 1 commit into from Nov 21, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jun 6, 2022

Description of changes

Since 97c4382 the file package has been part of stdenv, and no longer needs to be listed explicitly as a build input. Let's remove the platform-specific inclusion for mingw64 as suggested by @mehmooda:

#168413 (comment)

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
  • 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/)
  • Fits CONTRIBUTING.md.

Since 97c4382 the `file` package has
been part of stdenv, and no longer needs to be listed explicitly as a
build input.  Let's remove the platform-specific inclusion for mingw64
as suggested by @mehmooda:

  #168413 (comment)

I traced the line removed by this commit through the `git blame`; it
was initially added in this commit (and then shuffled around a few
dozen times by refactorings):

  8b292a1

The commit message indicates that `libpng-1.6.20` was current at the
time.  Although there are [libpng
archives](https://github.com/glennrp/libpng) available in git form,
the older versions don't have their autoconfery vendored in, so I
can't link to them.  Fortunately the relevant bit hasn't changed since
then:

https://github.com/glennrp/libpng/blob/a37d4836519517bdce6cb9d956092321eca3e73b/configure#L5575

```
mingw* | pw32*)
  # Base MSYS/MinGW do not provide the 'file' command needed by
  # func_win32_libid shell function, so use a weaker test based on 'objdump',
  # unless we find 'file', for example because we are cross-compiling.
  if ( file / ) >/dev/null 2>&1; then
    lt_cv_deplibs_check_method='file_magic ^x86 archive import|^x86 DLL'
    lt_cv_file_magic_cmd='func_win32_libid'
  else
    # Keep this pattern in sync with the one in func_win32_libid.
    lt_cv_deplibs_check_method='file_magic file format (pei*-i386(.*architecture: i386)?|pe-arm-wince|pe-x86-64)'
    lt_cv_file_magic_cmd='$OBJDUMP -f'
  fi
  ;;
```
@ghost
Copy link
Author

ghost commented Jun 7, 2022

pkgsCross.mingwW64.libpng builds for me with this commit.

However this commit plus git revert 77883ebe978a7fdafa0ab85fead1c69600b8c24e also builds, and produces the same number of files in the outdir. So I am unable to replicate the build failure which originally motivated the addition of the line.

Not sure how to proceed.

@Artturin Artturin requested a review from vcunat July 3, 2022 03:52
@ghost
Copy link
Author

ghost commented Oct 2, 2022

Not sure how to proceed.

I think the line removed by this commit was unnecessary even before file became part of stdenv.

@ghost ghost marked this pull request as ready for review October 2, 2022 00:15
@ghost ghost requested review from Ericson2314 and matthewbauer as code owners October 2, 2022 00:15
@Artturin Artturin merged commit df3056b into NixOS:staging Nov 21, 2022
@ghost ghost deleted the pr/remove-redundant-file-buildInput branch November 22, 2022 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant