Skip to content

bzip2: fix cross build on mingw by using autoconf patch#10820

Closed
mcmtroffaes wants to merge 1 commit intoNixOS:masterfrom
mcmtroffaes:feature/bzip2-mingw
Closed

bzip2: fix cross build on mingw by using autoconf patch#10820
mcmtroffaes wants to merge 1 commit intoNixOS:masterfrom
mcmtroffaes:feature/bzip2-mingw

Conversation

@mcmtroffaes
Copy link
Copy Markdown
Contributor

This also removes most of the sed script fixes for the cross builds: autoconf knows the right tools, and also knows not to run the tests. Tested with:

with import <nixpkgs> {
  crossSystem = {
    config = "i686-w64-mingw32";
    arch = "i686";
    libc = "msvcrt";
    platform = { };
    openssl.system = "mingw";
  };
};
{
  my-env = stdenv.mkDerivation {
    name = "my-env";
    buildInputs = [ bzip2.crossDrv ];
  };
}

@mcmtroffaes
Copy link
Copy Markdown
Contributor Author

cc @aszlig as nix expert concerning the mingw cross build target (judging from the nixpkgs git history)

@aszlig aszlig self-assigned this Nov 16, 2015
@mcmtroffaes
Copy link
Copy Markdown
Contributor Author

Yes, you're correct in that it only overrides those phases. The problem is this line:

https://github.com/NixOS/nixpkgs/blob/0ceda11/pkgs/tools/compression/bzip2/builder.sh#L7

and Makefile-libbz2_so is completely broken on cross targets. Other cross builds (such as on Fedora) use the autoconf patch to get their cross build running, see for instance: http://pkgs.fedoraproject.org/cgit/mingw-bzip2.git/tree/mingw-bzip2.spec#n19 so I'm following what appears to be best practice concerning cross builds of bzip2.

It entered my mind to use the autoconf patch on all targets (both native and cross) and to chuck out the old method, but of course that will cause a mass rebuild as a lot of stuff depends on this package. But if it is felt that this is better, I can rework the patch along those lines. I'm open to other suggestions too to make bzip2 work on cross targets.

aszlig added a commit that referenced this pull request Nov 17, 2015
Everything the builder.sh did can be done with the generic builder which
makes it easier to override attributes and also easier to read.

The reason I've done this is because of #10820, which tries to override
the preBuild hook, but the latter is hardcoded in the builder.sh of
bzip2.

I have compared the output of this against the previous version and the
only things that were different were timestamps in libbz2.a.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@aszlig
Copy link
Copy Markdown
Member

aszlig commented Nov 17, 2015

I've removed the builder.sh in 96648a8 (currently in staging branch), can you rebase your work on top of that?

@mcmtroffaes
Copy link
Copy Markdown
Contributor Author

Thanks a lot for the work. Yep, I will do a rebase on top of staging and send a new pull request against staging (I'll close this one once that's happened).

@peti
Copy link
Copy Markdown
Member

peti commented Nov 17, 2015

Is there any way to download that patch from some other place? It would be nice if we wouldn't have to check that (large'ish) patch into the Nixpkgs repository.

@mcmtroffaes
Copy link
Copy Markdown
Contributor Author

@peti
Copy link
Copy Markdown
Member

peti commented Nov 17, 2015

Any of those places seem fine to me. What matters is that the patch is retrieved with fetchurl rather than being checked into the repository.

@mcmtroffaes
Copy link
Copy Markdown
Contributor Author

No problem at all, I will do a fetchurl on the next iteration.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Nov 18, 2015

Right, so this should be closed, as the next iteration is to be submitted against staging. (Github seems not to allow changing target branch in PR.)

@vcunat vcunat closed this Nov 18, 2015
@mcmtroffaes
Copy link
Copy Markdown
Contributor Author

Yep, sadly it doesn't...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants