[vcpkg-make] new script port by Neumann-A · Pull Request #33198 · microsoft/vcpkg

Neumann-A · 2023-08-15T13:15:15Z

Just initial thoughts.

@dg0yt: Do you have more ideas for make related functions which might be useful?

dg0yt

I wonder if we could let the configure step generate a special Makefile (or a set of such files).
This file would contain the common environment variable setup, the call to configure including all options, etc.
It could serve as a pseudo log file in failure logs.
It could enable parallel configuration which would reduce build time.

ports/vcpkg-make/vcpkg-make.cmake

dg0yt · 2023-08-18T06:57:04Z

ports/vcpkg-make/vcpkg-make.cmake

+    cmake_parse_arguments(PARSE_ARGV 0 arg
+        "AUTOCONFIG;COPY_SOURCE;USE_WRAPPERS;NO_WRAPPERS;NO_CPP;DETERMINE_BUILD_TRIPLET"
+        "SOURCE_PATH;CONFIGURE_SUBPATH;BUILD_TRIPLET"
+        "OPTIONS;OPTIONS_DEBUG;OPTIONS_RELEASE;CONFIGURE_ENVIRONMENT_VARIABLES;CONFIG_DEPENDENT_ENVIRONMENT;ADDITIONAL_MSYS_PACKAGES"


I would like to add a CONFIGURE_COMMAND option, to invoke stuff like openssl's python script.
Maybe this idea needs more work to filter/substitute parameters.

Why does it use vcpkg_configure_make then? (But I think this is too port specific, you probably want to have the same environment/options setup as vcpkg_configure_make but actually want to do it a little different.)

It is a configure script which is similar enough to not duplicate 95% of the code.

ports/vcpkg-make/vcpkg-make-common.cmake

ports/vcpkg-make/vcpkg-make.cmake

ports/vcpkg-make/vcpkg_make_configure.cmake

…vcpkg-make # Conflicts: # ports/vcpkg-make/vcpkg-make-common.cmake # ports/vcpkg-make/vcpkg-make.cmake

Neumann-A · 2024-04-30T09:14:31Z

@JavierMatosD: Did I miss something to be removed?

I am not removing COPY_SOURCE. There are 18 ports using it with vcpkg_configure_make
SHELL is currently not used. It was added as a result of CR removing ADDITIONAL_MSYS_PACKAGES (at least 5 users)
OPTIONS_DEBUG/OPTIONS_RELEASE but is actively used by ports using vcpkg_configure_make and is parallel to other buildsystem scripts

JavierMatosD · 2024-05-01T15:03:05Z

@JavierMatosD: Did I miss something to be removed?

I am not removing COPY_SOURCE. There are 18 ports using it with vcpkg_configure_make

SHELL is currently not used. It was added as a result of CR removing ADDITIONAL_MSYS_PACKAGES (at least 5 users)

OPTIONS_DEBUG/OPTIONS_RELEASE but is actively used by ports using vcpkg_configure_make and is parallel to other buildsystem scripts

Can we migrate enough ports to use vcpkg_make_configure such that these options are actually being used in this PR?

Neumann-A · 2024-05-01T19:41:24Z

CI errors here are baseline regressions: #38509

Can we migrate enough ports to use vcpkg_make_configure such that these options are actually being used in this PR?

The whole idea here was not to port that many ports and keep it focused on the script. I could add the options unnecessarily to an already updated port.
libidn2 could be ported for everything except SHELL. SHELL would require something like hunspell/gettext which I don't want to touch.

JavierMatosD · 2024-05-13T17:46:58Z

SHELL would require something like hunspell/gettext which I don't want to touch.

Can we remove it for now then? We can always add it on later when it's needed.

Neumann-A · 2024-05-13T18:01:42Z

Can we remove it for now then? We can always add it on later when it's needed.

I can remove it from the top level but not from the lower level functions since it is basically this:

    if(NOT arg_SHELL)
      vcpkg_make_get_shell(arg_SHELL)
    endif()
    set(shell_cmd "${arg_SHELL}")

dg0yt · 2024-05-31T06:58:45Z

vcpkg_configure_make redirects tools to tools/<port>[/debug]/bin.
Can this be changed to tools/<port>[/debug]?
Of course it adds more changes where other steps are tightly coupled to the actual path.

dg0yt · 2024-06-01T18:53:45Z

ports/vcpkg-make/vcpkg_make.cmake

+        "PACKAGES"
+    )
+    z_vcpkg_unparsed_args(FATAL_ERROR)
+    list(APPEND msys_require_packages autoconf-wrapper automake-wrapper binutils libtool make which)


IMO Windows/msys should behave more like other platforms. We have several ports which need and either have or have not

vcpkg/ports/icu/portfile.cmake

Lines 1 to 14 in 47364fb

if(NOT VCPKG_TARGET_IS_WINDOWS)

message(WARNING "${PORT} currently requires the following programs from the system package manager:

autoconf automake autoconf-archive

On Debian and Ubuntu derivatives:

sudo apt-get install autoconf automake autoconf-archive

On recent Red Hat and Fedora derivatives:

sudo dnf install autoconf automake autoconf-archive

On Arch Linux and derivatives:

sudo pacman -S autoconf automake autoconf-archive

On Alpine:

apk add autoconf automake autoconf-archive

On macOS:

brew install autoconf automake autoconf-archive\n")

endif()

IMO this should be very much centralized to this script port, for
autoconf (incl. autoconf-archive), automake, make libtool.

Ideally, the port would also check for the presence of the required tools, and not print warnings if everything is installed.

With that perspective, it must be discussed if these packages can be requested unconditionally, or if it should be subject to input parameters (AUTOCONFIG, SYSTEM_PACKAGES).

dg0yt · 2024-06-01T18:55:19Z

ports/vcpkg-make/vcpkg_make_configure.cmake

+    cmake_parse_arguments(PARSE_ARGV 0 arg
+        "AUTOCONFIG;COPY_SOURCE;DISABLE_MSVC_WRAPPERS;NO_CPPFLAGS;NO_DEFAULT_OPTIONS;NO_MSVC_FLAG_ESCAPING;USE_RESPONSE_FILES"
+        "SOURCE_PATH"
+        "OPTIONS;OPTIONS_DEBUG;OPTIONS_RELEASE;PRE_CONFIGURE_CMAKE_COMMANDS;LANGUAGES"


I notice that ADDITIONAL_MSYS_PACKAGES is gone, but I don't see how to pass an extended msys environment.

Option SHELL was removed with a94f56b since it was not used yet by any port. @JavierMatosD made me remove it so feel free to argue with him

SHELL was not used, maybe it didn't work.
SKIP_CONFIGURE didn't work most of the time, the workaround aren't visible.
ADDITIONAL_MSYS_PACKAGES is used, and it is gone nevertheless.

SHELL was not used, maybe it didn't work.

SHELL works since it is used internally. Was just an option passed down and otherwise defaulted to the internal msys shell.

ADDITIONAL_MSYS_PACKAGES is used, and it is gone nevertheless.

Its gone due to: #33198 (comment)

The proposal was to USE_MSYS_ROOT which is fine from my POV.
You wrote "replaced with SHELL" but this was removed for not being used before it became the alternative for ADDITIONAL_MSYS_PACKAGES.

And that's why there is a gap now.

(But I want a generic ADDITIONAL PACKAGES.)

I replaced it with shell since that is more universal and what is actual required by the scripts to run. (and is not limited to windows.)

not being used before

No that is not the reason why it got removed. It got removed because no port using vcpkg-make here in this PR is yet using it. You can add whatever further options to vcpkg-make later if you have changes in ports requiring it. Yeah its a bit annoying however it is better to move forward instead of forcing and delaying the script port longer.

Consider what they said about your changes to vcpkg_configure_make and why they were blocked. This is a script port so you can change it however you like later. So the question for you right know should be: Is it a good basis to build upon later even if you right now don't get all the features you would like to have?

This is a script port so you can change it however you like later.

No. We can only make changes which don't break existing packages. Because only one version can be installed, and user mix consuming ports in arbitrary ways.

Is it a good basis to build upon later even if you right now don't get all the features you would like to have?

I can't tell.

What I can do if I'm not happy with it is to create another port. Or suggest an "API level" switch like vcpkg_make_minimum_required(VERSION 2.0) or vcpkg_minimum_required(vcpkg-make 2.0).

user mix consuming ports in arbitrary ways.

Problem of the user currently if they are not following a baseline.
Later it might be on the tool to inject the vcpkg-scriptport which is on the same baseline as the port requesting it but that is currently not on the discussion table.

Or suggest an "API level" switch.

Quite easy to implement

function(vcpkg_configre_make) cmake_parse_arguments (arg ... ... "VERSION" ...) cmake_language(CALL vcpkg_configre_make_${arg_VERSION} ${arg_UNPARSED_ARGUMENTS}) endfunction(vcpkg_configre_make)

but only really required if there is a breaking change.

Neumann-A · 2024-06-01T19:00:41Z

vcpkg_configure_make redirects tools to tools/[/debug]/bin.
Can this be changed to tools/[/debug]?
Of course it adds more changes where other steps are tightly coupled to the actual path.

I think all tools should be moved to prefix/bin instead of prefix. This way tools/<port>[/debug]? can just be understood as a different install prefix instead of a totally different thing. (I already had problem with tools not being in a bin subdir.)

dg0yt · 2024-06-01T19:06:55Z

I think all tools should be moved to prefix/bin instead of prefix.

There are other places to discuss why it must be bin and not tools.
The only question I raise here is ig autotools build can use the same prefix as cmake builds.
Not to mention that there needs to be a `vcpkg_copy_tool_dependencies.

(And hey, you invented manual-tools instead of libexec.)

Neumann-A · 2024-06-01T19:35:21Z

There are other places to discuss why it must be bin and not tools.
The only question I raise here is ig autotools build can use the same prefix as cmake builds.
Not to mention that there needs to be a `vcpkg_copy_tool_dependencies.

I think you misunderstood. I mean prefix=vcpkg_installed/tools/${PORT} (I don't directly mean the option --prefix with that)
The idea behind vcpkg_installed/tools/${PORT}/bin is that vcpkg_installed/tools/${PORT} can be understood as the prefix for the tool. Debug tools should probably go to vcpkg_installed/debug/tools/${PORT}/bin but that is ultimately for Billy to figure out (although it has already been over a year).

(And hey, you invented manual-tools instead of libexec.)

manual-tools is not a default search path of anything. (which was the main idea.) libexec is probably also some sort of search dir for CMake or other stuff. But I don't mind changing that to something which is shorter and also makes sense.

Regarding *-config scripts. Those should probably really be kept in vcpkg_installed/(debug/)?bin (since then cmake should find the correct one without problems.) But that is a different discussion.

dg0yt · 2024-06-01T20:15:42Z

Okay, then let's say that vcpkg_installed/tools/${PORT} is the prefix for a "bundle", with executables inside bin.
Only that our bundles don't include runtime libs except for Windows. And even not all of them for Windows. (Looking at Qt plugins and their runtime deps.)

manual-tools is not a default search path of anything. (which was the main idea.)

And this is [/usr/]libexec: "Binaries run by other programs that are not intended to be executed directly by users or shell scripts (optional).", https://en.wikipedia.org/wiki/Filesystem_Hierarchy_Standard
CMake must not search there.

that is ultimately for Billy to figure out (although it has already been over a year).

It feels like three years and one month with #17607. Not counting #13649.

dg0yt · 2024-06-02T11:15:08Z

ports/vcpkg-make/vcpkg_make_install.cmake

+        list(APPEND arg_OPTIONS_DEBUG ${VCPKG_MAKE_OPTIONS_DEBUG})
+    endif()
+
+    if(WIN32)


Host or target? Ambiguous variable, please replace.

Waiting for JavierMatosD before applying changes. (you know there is this other PR where he seems to be testing stuff.)

Watching every detail ;-)
(you know there is this other PR about catching ambiguous variables.)

dg0yt · 2024-06-05T17:38:37Z

Now I remember why SKIP_CONFIGURE was barely usable:
It wasn't (easily) possible to pass all required options (per build type) into the make install step. That's why I add a small configure script to some ports. It creates a Makefile.vcpkg which captures the configuration options and passed them on to the actual Makefile. Example: libcap.

Neumann-A · 2024-06-05T17:48:25Z

#38490 (comment) I remember why SKIP_CONFIGURE was barely usable:
It wasn't (easily) possible to pass all required options (per build type) into the make install step. That's why I add a small configure script to some ports. It creates a Makefile.vcpkg which captures the configuration options and passed them on to the actual Makefile. Example: libcap.

I think I changed that to just not use a vcpkg_make_configure command. vcpkg_make_(build|install) will then call make with the env setup.

dg0yt · 2024-06-13T06:16:21Z

Seeing https://github.com/microsoft/vcpkg/pull/39261/files#diff-6149f0940da9ef457033a1e08359d24ef1654f81b469e8bcf81cb41c2d10969eR13-R21:

We still need some transformation between FEATURES and --with-foo and --enable-foo options to configure.
Either another function vcpkg_make_check_features, or direct options in vcpkg_make_configure.

Neumann-A · 2024-06-13T20:16:10Z

@JavierMatosD Should I close this PR?

JavierMatosD · 2024-06-13T21:33:05Z

@JavierMatosD Should I close this PR?

Your call :) I've added a test port with some unit tests for various functions and cleaned up some of the code (removing comments, extracting functions, etc). I was going to try and figure out the libidn2 issue as well before kicking it off to the team for a final review. If you prefer, you can take my changes and we can continue here.

Neumann-A · 2024-06-13T21:42:49Z

Nah just lets take #39050. Migrating the changes back here is just extra work and time. Please integrate #33198 (comment) into #39050 (change the line to use CMAKE_HOST_WIN32)

We still need some transformation between FEATURES and --with-foo and --enable-foo options to configure.
Either another function vcpkg_make_check_features, or direct options in vcpkg_make_configure.

Can be added in a later PR.

Neumann-A added 2 commits August 15, 2023 15:11

vcpkg-make initial idea

b499ded

don't run ci

62eb284

LilyWangLL assigned JonLiu1993 Aug 16, 2023

JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 16, 2023

Neumann-A added 3 commits August 16, 2023 23:32

Make vcpkg portfile execution debugable

fffa564

WIP

2e1d32a

more WIP

36ce65c

dg0yt reviewed Aug 18, 2023

View reviewed changes

Neumann-A added 7 commits August 18, 2023 10:26

install wrappers

23d3308

keep the configure_make somewhere for reference

92457bf

add function to warn about spaces

ab57ae7

strip whitespaces

7918e80

WIP

d3b8148

determine configure triplet

1f56817

wip

3ee0aa0

dg0yt reviewed Aug 18, 2023

View reviewed changes

Neumann-A added 5 commits August 18, 2023 23:07

WIP

799b4c8

WIP . move more stuff over.

eaf0b67

WIP: create vcpkg_make_install

c0dc7d7

Merge remote-tracking branch 'upstream/master' into vcpkg-make

c5381ab

WIP code tuning

3933374

Neumann-A commented Aug 25, 2023

View reviewed changes

ports/vcpkg-make/vcpkg_make_configure.cmake Outdated Show resolved Hide resolved

Neumann-A added 8 commits August 25, 2023 17:34

rename function to z_vcpkg_make_get_configure_triplets

3952202

Merge branch 'vcpkg-make' of https://github.com/Neumann-A/vcpkg into …

407d3f8

…vcpkg-make # Conflicts: # ports/vcpkg-make/vcpkg-make-common.cmake # ports/vcpkg-make/vcpkg-make.cmake

add configure cache

0b0df8d

More wip

facb152

fix includes

55ab880

fix automatic file include

fb6f0b0

make gmp use the new stuff

1bf57d6

fix stuff to configure (build flags are missing somehow)

a3621d3

Neumann-A added 4 commits April 29, 2024 22:19

remove ADD_BIN_TO_PATH

39d86c1

remove POST_CONFIGURE_CMAKE_COMMANDS

dfb60ab

remove some comments

cd156fe

v db

e590d18

Neumann-A marked this pull request as ready for review April 30, 2024 09:14

Merge remote-tracking branch 'upstream/master' into vcpkg-make

4b53ef9

Neumann-A added 2 commits May 1, 2024 21:48

port libidn2 to vcpkg-make

e528798

v db

7af0699

Neumann-A added 3 commits May 15, 2024 22:41

Merge remote-tracking branch 'upstream/master' into vcpkg-make

940d428

Remove SHELL as a toplevel option for now

a94f56b

v db

0b8b6cc

dg0yt reviewed Jun 1, 2024

View reviewed changes

dg0yt reviewed Jun 2, 2024

View reviewed changes

Neumann-A closed this Jun 13, 2024

	if(NOT VCPKG_TARGET_IS_WINDOWS)
	message(WARNING "${PORT} currently requires the following programs from the system package manager:
	autoconf automake autoconf-archive
	On Debian and Ubuntu derivatives:
	sudo apt-get install autoconf automake autoconf-archive
	On recent Red Hat and Fedora derivatives:
	sudo dnf install autoconf automake autoconf-archive
	On Arch Linux and derivatives:
	sudo pacman -S autoconf automake autoconf-archive
	On Alpine:
	apk add autoconf automake autoconf-archive
	On macOS:
	brew install autoconf automake autoconf-archive\n")
	endif()

Conversation

Neumann-A commented Aug 15, 2023

Uh oh!

dg0yt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Neumann-A commented Apr 30, 2024

Uh oh!

JavierMatosD commented May 1, 2024

Uh oh!

Neumann-A commented May 1, 2024

Uh oh!

JavierMatosD commented May 13, 2024

Uh oh!

Neumann-A commented May 13, 2024

Uh oh!

dg0yt commented May 31, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt Jun 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neumann-A Jun 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neumann-A commented Jun 1, 2024

Uh oh!

dg0yt commented Jun 1, 2024

Uh oh!

Neumann-A commented Jun 1, 2024

Uh oh!

dg0yt commented Jun 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Jun 5, 2024

Uh oh!

Neumann-A commented Jun 5, 2024

Uh oh!

dg0yt commented Jun 13, 2024

dg0yt Jun 1, 2024 •

edited

Loading

Neumann-A Jun 1, 2024 •

edited

Loading