[vcpkg_acquire_msys] Add gzip to default packages

[wrobelda]

Describe the pull request
Adds gzip to the list of default msys environment. gzip is used by autopoint, provided by gettext tools.

• What does your PR fix?

  Fixes [gettext] autopoint fails on Windows with missing gzip  #20392

• Which triplets are supported/not supported? Have you updated the CI baseline?

  No change.

• Does your PR follow the maintainer guide?

  Yes.

• If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

  N/A

[JackBoosY]

Gzip is not required by most ports, I think you should pass ADDITIONAL_MSYS_PACKAGES=gzip into vcpkg_configure_make.

[wrobelda]

Gzip is not required by most ports, I think you should pass ADDITIONAL_MSYS_PACKAGES=gzip into vcpkg_configure_make.

The thing is this is not a port requirement, but the autopoint provided by gettext[tools] needs it (in some cases?). Adding it to a port that depends on gettext's autopoint is a workaround, not a solution.

[JackBoosY]

Gzip is not required by most ports, I think you should pass ADDITIONAL_MSYS_PACKAGES=gzip into vcpkg_configure_make.

The thing is this is not a port requirement, but the autopoint provided by gettext[tools] needs it (in some cases?). Adding it to a port that depends on gettext's autopoint is a workaround, not a solution.

Ahh, yes. However, you should add gzip to vcpkg_configure_make here:

vcpkg/scripts/cmake/vcpkg_configure_make.cmake

Line 309 in 8e0a801

vcpkg_acquire_msys(MSYS_ROOT PACKAGES ${MSYS_REQUIRE_PACKAGES} ${_csc_ADDITIONAL_MSYS_PACKAGES})

Add the following code below:

    if (_csc_AUTOCONFIG)
        list(APPEND MSYS_REQUIRE_PACKAGES gzip)
    endif()

[wrobelda]

Ahh, yes. However, you should add gzip to vcpkg_configure_make here:

Thanks, that makes sense. Will fix it and let you know.

[wrobelda]

Thanks, that makes sense. Will fix it and let you know.

@JackBoosY fixed now.

[JackBoosY]

The libosip2:x86-windows regression is not related to this changes.

[JackBoosY]

@dg0yt @Neumann-A @cenit Are there any objections?

[dg0yt]

I would suggest to add the package without condition. The condition causes two different msys roots on disk, for a minimal gain in downloads for some ports - or no gain at all with a look at a realistic set of dependencies.

[JackBoosY]

@dg0yt I think we should find other ways to avoid the existence of duplicate msys roots, instead of adding most of the components that are not required by the port to the default components, because most users will not use it.

[Neumann-A]

I think we should add alle the m4 stuff als separate ports in the future and don't depend on msys so much.

[dg0yt]

There is too much "in the future" in the last comments. For this PR now, my recommendation is to remove the condition. Say yes and no, so that the author knows how to proceed.

[cenit]

i agree with @dg0yt . For now, better to have the component without conditions. We can move to m4 ports later on, without any additional issue if we have the tool available right now thanks to msys

[JackBoosY]

Fine, very equal, I accept the majority opinion.
@wrobelda Please revert it and set it into vcpkg_acquire_msys.
Sorry for that.

[wrobelda]

Fine, very equal, I accept the majority opinion.
@wrobelda Please revert it and set it into vcpkg_acquire_msys.
Sorry for that.

Nothing to be sorry about. Glad there's a consensus. I restored the previous version.

[strega-nil-ms]

LGTM!

[BillyONeal]

Should we remove MSYS_ROOT PACKAGES gzip and ADDITIONAL_MSYS_PACKAGES gzip where they exist in ports? @cenit @JackBoosY @wrobelda

[ras0219-msft]

The thing is this is not a port requirement, but the autopoint provided by gettext[tools] needs it (in some cases?). Adding it to a port that depends on gettext's autopoint is a workaround, not a solution.

I think this is a bit more nuanced; just to confirm:

1. gettext[tools] produces autopoint which is a shell script1 that wants to shell out to gzip.exe
2. Port $X depends on gettext[tools]:$host to get autopoint

In an ideal world, $X wouldn't need to know about the gzip dependency at all. It wants autopoint provided by a port, which means we would want any transitive dependencies of autopoint to be managed by that port.

Perhaps this would be a vcpkg-port-config.cmake for gettext[tools] that runs vcpkg_acquire_msys2(GZIP_ROOT PACKAGES gzip NO_DEFAULT_PACKAGES) and then adds it to the path.

Looking at the source of autopoint.in1, I see three hits for gzip, all of which are easily matched by the regex " gzip ". Therefore, it would also be viable to do something along the lines of:

vcpkg_acquire_msys2(GZIP_ROOT PACKAGES gzip NO_DEFAULT_PACKAGES)
file(INSTALL "${GZIP_ROOT}" DESTINATION ${CURRENT_PACKAGES_DIR}/tools/${PORT}/bin RENAME gzip)
vcpkg_replace_string(${CURRENT_PACKAGES_DIR}/tools/${PORT}/bin " gzip " " \"$bindir/gzip/bin/gzip\" ")

This ensures that the package remains portable and fully encapsulated.

[dg0yt]

Perhaps this would be a vcpkg-port-config.cmake for gettext[tools] that runs vcpkg_acquire_msys2(GZIP_ROOT PACKAGES gzip NO_DEFAULT_PACKAGES) and then adds it to the path.

Looking at the source of autopoint.in1, I see three hits for gzip, all of which are easily matched by the regex " gzip ". Therefore, it would also be viable to do something along the lines of:

vcpkg_acquire_msys2(GZIP_ROOT PACKAGES gzip NO_DEFAULT_PACKAGES)
file(INSTALL "${GZIP_ROOT}" DESTINATION ${CURRENT_PACKAGES_DIR}/tools/${PORT}/bin RENAME gzip)
vcpkg_replace_string(${CURRENT_PACKAGES_DIR}/tools/${PORT}/bin " gzip " " \"$bindir/gzip/bin/gzip\" ")

This ensures that the package remains portable and fully encapsulated.

Note that this may need more mingw/msys runtime environment setup than just copying of the contents of gzip. It seems to me that this leads into another source of fragility with regard to mingw/msys runtime.

If you want an alternative to gzip, there is cmake -E tar.

[ras0219-msft]

Note that this may need more mingw/msys runtime environment setup than just copying of the contents of gzip

My logic in the above is that we are copying the entire layout provided by vcpkg_find_acquire_msys() for gzip. This should include all dependencies needed to run gzip (currently just msys2-runtime) -- otherwise that's a bug in vcpkg_find_acquire_msys(). I think I have the wrong precise subpath for the gzip executable though. It would more likely be $bindir/gzip/usr/bin/gzip.

[dg0yt]

gzip download size is 104 KB.

vcpkg_acquire_msys2(GZIP_ROOT PACKAGES gzip NO_DEFAULT_PACKAGES)

Indeed this pulls in msys2-runtime. It still creates another msys root, with / mapping to another windows path. I'm really not happy about more and more msys roots.

[JackBoosY]

@dg0yt How to extend an msys root? What's the problem with this?

[dg0yt]

@JackBoosY I'm actually asking for extending the default msys root. Because I'm opposed to an additional msys root.

Every different set of packages creates another msys root. The msys root directory path is based on the hash of the set of packages. A default autotools build already needs two msys roots, due to pkg-config being acquired separately.

[JackBoosY]

@dg0yt I personally agree with this, we should use the directory with the updated date instead of the hash that contains all the extensions.
But if we let all ports use the same msys root, will we encounter problems when expanding?

[Neumann-A]

Just switch everything to vcpkg-msys-* ports. Then there is one root for msys

[dg0yt]

Just switch everything to vcpkg-msys-* ports. Then there is one root for msys

@Neumann-A As discussed before, that is a long-term perspective. (Who will do it, and when?) ATM we can do only a minimal change. Not to mention the unfinished script audit.

[Neumann-A]

the reason different msys roots have been introduced is to find hidden dependency issues with msys which constantly broke CI.

[wrobelda]

Is there some consensus – or a compromise at least – on how to proceed here for the time being?

[JackBoosY]

Ping @ras0219-msft for reply.

[ras0219-msft]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[ras0219-msft]

While I do still think my suggested approach above of encapsulating the gzip dependency would be the best option, gzip is small enough and likely enough to be commonly needed that it seems acceptable to add to the base installation. This is a unique case and is not intended to set a precedent.

[BillyONeal]

Thanks!