Skip to content

vcpkg_configure_make: mingw fix: Use the vcpkg-acquired msys binaries for objdump / nm#18055

Closed
past-due wants to merge 1 commit intomicrosoft:masterfrom
past-due:mingw_objdump_fix_1
Closed

vcpkg_configure_make: mingw fix: Use the vcpkg-acquired msys binaries for objdump / nm#18055
past-due wants to merge 1 commit intomicrosoft:masterfrom
past-due:mingw_objdump_fix_1

Conversation

@past-due
Copy link
Contributor

@past-due past-due commented May 21, 2021

Describe the pull request

  • mingw: Use the vcpkg-acquired msys binaries for objdump / nm
    • Otherwise mingw environments that use LLVM-based toolchains (example: the msys2 clang64, clang32 environments) cannot build ports as their objdump is preferred and its output differs enough that it isn't properly parsed by libtool.

Cherry-picked from #18028

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

    only mingw triplets are impacted

@Neumann-A
Copy link
Contributor

A few things:

  • should vcpkg even try to acquire MSYS in this case? With MinGW isn't there already a working MSYS environment? (or at least one which can run bash and so on)
  • Shouldn't the toolchain be adjusted in this cases instead of special casing in vcpkg_configure_make? Maybe a way to control lt_cv_deplibs_check_method=pass_all via the triplet is more appropiate?

@past-due
Copy link
Contributor Author

past-due commented May 23, 2021

A few things:

  • should vcpkg even try to acquire MSYS in this case? With MinGW isn't there already a working MSYS environment? (or at least one which can run bash and so on)

I was seeing issues in the clang msys2 environments when the actual environment (LLVM-based) tools were being selected. (Hence relying on the acquired msys, which is guaranteed to have the gnu tools, and thus have more consistent behavior between regular Windows triplets and mingw triplets without relying on specifics of the potential msys environment.)

  • Shouldn't the toolchain be adjusted in this cases instead of special casing in vcpkg_configure_make? Maybe a way to control lt_cv_deplibs_check_method=pass_all via the triplet is more appropiate?

This seems like the better place to make the fix IMHO because it is only required for make/automake builds. I debated the latter measure, but this proposal does not require bypassing the libtool checks.

@JackBoosY JackBoosY added category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:world-rebuild labels May 24, 2021
@JackBoosY
Copy link
Contributor

Does anyone object to this change?

Copy link
Contributor

@JackBoosY JackBoosY left a comment

Choose a reason for hiding this comment

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

One thing I need to confirm: why objdump and nm are used in the build process?

@dg0yt
Copy link
Contributor

dg0yt commented May 24, 2021

objdump and nm are used by libtool which deals with building shared libraries correctly.

I'm not fully convinced if there is a point in extending vcpkg_acquire_msys from providing the generic POSIX tools (for windows triplets) to providing toolchain components (for mingw triplets). In particular when the user probably comes from a more complete mingw environment anyway (MSYS2).

@JackBoosY JackBoosY added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label May 24, 2021
@past-due
Copy link
Contributor Author

After discussion in the vcpkg Discord, closing this in favor of #18132

@past-due past-due closed this May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants