[vcpkg_configure/build_make] Fix variable setup

[dg0yt]

• What does your PR fix?

  Fixes difficulties to set (environement) variables and make variables.

  • vcpkg_configure/build_make: Extend CONFIGURE_ENVIRONMENT_VARIABLES setup from windows targets to all platforms. (Documentation mentions no restrictions.)
  • vcpkg_build_make: Fix OPTIONS for non-windows hosts. Drop invalid MAKE_OPTIONS.

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

  unchanged, no

• 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?

  not needed

[dg0yt]

CC @Neumann-A

I already knew the make options issue. With port x264 (which is not using autotools), I really need to have consistent behaviour if I want to be able improve cross build support.

[JackBoosY]

Note to other reviewer: moved these code out of VCPKG_TARGET_IS_WINDOWS.

[JackBoosY]

If anyone does not object to this change, I will mark the reviewed tag later today.

[Neumann-A]

Creating the environment string should probably be moved into its own function. But i don't object the movement just make sure the other make PR gets merged around the same time to minimize world rebuilds.

[dg0yt]

I found that there is still more Windows-only behaviour going on in with vcpkg_configure_make.

I want to go one step back and fully understand the current intention of CONFIGURE_ENVIRONMENT_VARIABLES (which is behind VCPKG_TARGET_IS_WINDOWS ATM) and z_vcpkg_append_to_configure_environment (which has a CMAKE_HOST_WIN32 ATM).

AFAIU it is related to the following:

• For Windows hosts,
  vcpkg tool clears most environment variables. This happens before calling CMake.
• For Windows targets,
  vcpkg_configure_make initializes well-known autotools environment variables by injecting them to the bash command line via z_vcpkg_append_to_configure_environment.
• For Windows targets,
  vcpkg_configure_make prepends the parent dirs of well-known tools detected by the CMake toolchain to the PATH.
  This allows tools to be executed via basename and avoids problems with space characters due to C:\Program Files.
• For Windows hosts,
  z_vcpkg_append_to_configure_environment allows to override detected toolchain settings with the current environment variable value. (The environment variable is meant to come from the triplet.)
• For Windows targets,
  CONFIGURE_ENVIRONMENT_VARIABLES is a parameter which allows to inject additional environment variables to the bash command line, with a value from CMake variable (assumed to be set in the portfile),
  for Windows hosts
  possibly overriden by the environment variable.

Unfortunately, this is quite inconsistent, depending on whether target and/or host are windows.

Example: Targeting ENV{AS} in port x264:

Current master, without CONFIGURE_ENVIRONMENT_VARIABLES:

ENV{AS} in	Target:	android	windows	mingw	mingw	linux
	Host:	windows	windows	mingw	linux	linux
unset	ENV{AS} out:	unset	:	:	:	unset
set	ENV{AS} out:	ENV{AS}	ENV{AS}	ENV{AS}	: 🟥	ENV{AS}

🟥 Problem: ENV{AS} still uses the default : for mingw cross builds.

Current master, with CONFIGURE_ENVIRONMENT_VARIABLES AS:

ENV{AS} in	Target:	android	windows	mingw	mingw	linux
	Host:	windows	windows	mingw	linux	linux
unset	ENV{AS} out:	unset	AS	AS	AS	unset
set	ENV{AS} out:	ENV{AS}	ENV{AS}	ENV{AS}	AS 🟥	ENV{AS}

🟥 Problem: ENV{AS} still defined by AS for mingw cross builds.

A consistent pattern

I assuming that for setting well-known autotools variables in vcpkg_configure_make, we have to deal with default values, overrides from a portfile, and overrides from a (custom) triplet. A possible consistent pattern is:

1. The default values for setting a well-known environment variable ENV{<VAR>} is derived from the CMake toolchain, if needed.
2. An alternative value can be set from the portfile by setting CMake variable <VAR> and passing CONFIGURE_ENVIRONMENT_VARIABLES <VAR>.
3. An alternative to default values and portfile values can be set from the triplet via ENV{<VAR>}.

Changes

a. Don't override ENV{<VAR>} for windows (mingw) targets on non-windows hosts (:red_square:).
(Required for no. 3. Basically, we can savely assume that if ENV{<VAR>} is meant to be used if it is already set.)
b. Implement CONFIGURE_ENVIRONMENT_VARIABLES also for other targets than Windows (this PR, but incomplete without a.).
(Required for no. 2. Current workaround is to override ENV{<VAR> in the portfile, but this may override triplet settings.)
c. Depending on on what is "needed", set the well-known autotools variables also for other triplets than windows.
(Required for consistency with regard to no. 1. However, we currently do very well without it, and it may come with side effects. For example, x264 can't handle a CC with space in the path.)
d. The current implementation may inject a variable twice into the bash command line (for the default and for CONFIGURE_ENVIRONMENT_VARIABLES). IMO this should be merged.
e. Why do we have to inject the variables into the bash command line at all? I would suggest to just set ENV{<VAR>} directly. If we are concerned about side effects on other CMake code, we may clear what we set before leaving.
f. Update ENV{PATH} for all platforms, maybe limited to tools which have space characters in the file path.

[LilyWangLL]

Ping @dg0yt for response. Is work still being done for this PR?

[dg0yt]

I wrote on long text on existing problems and possible directions. I'm still waiting for feedback.

[dg0yt]

So I stumble over this again now. If it is not possible to get feedback, I have to change my approach.

[BillyONeal]

So I stumble over this again now. If it is not possible to get feedback, I have to change my approach.

I think you aren't getting feedback because this is marked as "draft" which makes it not show up on any lists we look at except the "close really old draft PRs" one.

[BillyONeal]

(And also because this touches a bunch of autotools edge cases for which there is very little understanding among vcpkg's maintainers; as you point out with your tables above a lot of this is 'build and fix until it works' rather than careful design)

[dg0yt]

A) It is not ready for merge, and ATM I don't ask for a review of the implementation. But if needed, I can mark it "Ready for review". (I also have ready-for-review PRs which are stuck in review, #26754.)

B) ATM the issue is not an edge-case autotools particularity but a genuine vcpkg script question:
How to control the environment variables for the point where autotools are actually called?
It is the script which behaves inconsistenly. This can be resolved in various ways. I don't want to open a PR per alternative, but ask which one to choose.

[Neumann-A]

(which has a CMAKE_HOST_WIN32 ATM).

that probably should be a MSVC check instead (or CMAKE_C_COMPILER_FRONTEND_VARIANT)? When the first vcpkg_configure_make was written vcpkg_cmake_get_vars didn't yet exist so compiler checks weren't really an option.

Unfortunately, this is quite inconsistent, depending on whether target and/or host are windows.

--host-triplet didn't exist (and with that VCPKG_CROSSCOMPILING) when vcpkg_configure_make was created. So I mainly cared about native builds and get them working.

prepends the parent dirs of well-known tools

that is a requirement since most stuff cannot handle spaces at all.

injecting them to the bash command

that was required because from testing in seemed like ENV{PROG} wasn't always used which is why CONFIGURE_ENVIRONMENT_VARIABLES was introduced to inject extra stuff in the bash command.
Additionally I didn't want to trash ENV{CC} etc in the environment if the compile wrapper is used.
(Especially since the environment needs to persist beyond vcpkg_configure_make into vcpkg_install_make in some cases; maybe we need to pass the env string around for that)

The real problem is the amount of different make file buildsystem:

• pure Makefiles
• pure Makefiles using libtool
  (the above should probably always be converted to a CMakeLists.txt since ports using that are typically fairly simple)
• configure based without using autotools (custom configure)
• configure based using only autotools but not libtool/automake
• configure based full autotools but with custom m4 scripts
• configure based full autotools but with outdated m4 scripts / libtool
• configure based proper full autotools

So my advice here is just make it consistent with the smallest change set possible ;) The whole make system needs a general overhaul and be moved into a helper port any way. The env setup needs to be moved into its own little function so that it can be reused more often for other stuff.