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.

Sign up for GitHub

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

7zz: cross-compile for mingw #217472

Merged
merged 2 commits into from
Feb 21, 2023
Merged

7zz: cross-compile for mingw #217472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d3f8d98
7zz: cross-compile for mingw
AtnNn Feb 21, 2023
045e8b3
7zz: convert mingw script to patch
AtnNn Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions pkgs/tools/archivers/7zz/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,22 @@ stdenv.mkDerivation rec {

sourceRoot = ".";

patches = [ ./fix-build-on-darwin.patch ];
patches = [
./fix-build-on-darwin.patch
./fix-cross-mingw-build.patch
];
patchFlags = [ "-p0" ];

postPatch = lib.optionalString stdenv.hostPlatform.isMinGW ''
substituteInPlace CPP/7zip/7zip_gcc.mak C/7zip_gcc_c.mak \
--replace windres.exe ${stdenv.cc.targetPrefix}windres
'';

NIX_CFLAGS_COMPILE = lib.optionals stdenv.isDarwin [
"-Wno-deprecated-copy-dtor"
] ++ lib.optionals stdenv.hostPlatform.isMinGW [
"-Wno-conversion"
"-Wno-unused-macros"
];

inherit makefile;
Expand All @@ -73,7 +84,8 @@ stdenv.mkDerivation rec {
# aarch64-darwin so we don't need additional changes for it
++ lib.optionals stdenv.isDarwin [ "MACOSX_DEPLOYMENT_TARGET=10.16" ]
# it's the compression code with the restriction, see DOC/License.txt
++ lib.optionals (!enableUnfree) [ "DISABLE_RAR_COMPRESS=true" ];
++ lib.optionals (!enableUnfree) [ "DISABLE_RAR_COMPRESS=true" ]
++ lib.optionals (stdenv.hostPlatform.isMinGW) [ "IS_MINGW=1" "MSYSTEM=1" ];

nativeBuildInputs = lib.optionals useUasm [ uasm ];

Expand All @@ -84,7 +96,7 @@ stdenv.mkDerivation rec {
installPhase = ''
runHook preInstall

install -Dm555 -t $out/bin b/*/7zz
install -Dm555 -t $out/bin b/*/7zz${stdenv.hostPlatform.extensions.executable}
install -Dm444 -t $out/share/doc/${pname} ../../../../DOC/*.txt

runHook postInstall
Expand All @@ -109,7 +121,7 @@ stdenv.mkDerivation rec {
# the unRAR compression code is disabled by default
lib.optionals enableUnfree [ unfree ];
maintainers = with maintainers; [ anna328p peterhoeg jk ];
platforms = platforms.unix;
platforms = platforms.unix ++ platforms.windows;
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for working on this.

One question though. Is platforms.windows correct here?

nix-repl> lib.platforms.windows
[ "i686-cygwin" "x86_64-cygwin" "x86_64-windows" "i686-windows" ]

I don't know much about windows builds but it seems like Cygwin is a bit different to MinGW
Is "x86_64-cygwin" actually under isMinGW?

isCygwin       = { kernel = kernels.windows; abi = abis.cygnus; };
isMinGW        = { kernel = kernels.windows; abi = abis.gnu; };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cygwin and MinGW are indeed very different. I don't think it matters here because platforms.unix already includes Cygwin.

Copy link
Member

Choose a reason for hiding this comment

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

so it should have stayed platforms.unix and not added platforms.windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The two mingw doubles were needed, which platforms.windows provides.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Your question makes sense. Perhaps there should be a platforms.mingw, using that would have made the intention of the change clearer.

mainProgram = "7zz";
};
}
Loading