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.

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

[vcpkg_configure_make] Correct parameter SKIP_CONFIGURE #14402

Closed
wants to merge 4 commits into from

Conversation

JackBoosY
Copy link
Contributor

Ignore autogen and autoconfig judgment when using SKIP_CONFIGURE.

Fixes #14389.

@JackBoosY JackBoosY added info:internal This PR or Issue was filed by the vcpkg team. category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly labels Nov 5, 2020
@JackBoosY
Copy link
Contributor Author

cc @Neumann-A for review this PR.

@BillyONeal
Copy link
Member

Maybe we should skip all the AUTOCONF_REQUIRED etc. checks if SKIP_CONFIGURE is used? Something like:

diff --git a/scripts/cmake/vcpkg_configure_make.cmake b/scripts/cmake/vcpkg_configure_make.cmake
index bf1f93b9d..de01f29f2 100644
--- a/scripts/cmake/vcpkg_configure_make.cmake
+++ b/scripts/cmake/vcpkg_configure_make.cmake
@@ -180,13 +180,17 @@ function(vcpkg_configure_make)
 
     set(REQUIRES_AUTOGEN FALSE) # use autogen.sh
     set(REQUIRES_AUTOCONFIG FALSE) # use autotools and configure.ac
-    if(EXISTS "${SRC_DIR}/configure" AND "${SRC_DIR}/configure.ac") # remove configure; rerun autoconf
+    if (_csc_SKIP_CONFIGURE)
+        if (NOT EXISTS "${SRC_DIR}/Makefile")
+            message(FATAL_ERROR "vcpkg_configure_make was called with SKIP_CONFIGURE, but no makefile was found.")
+        endif()
+    elseif(EXISTS "${SRC_DIR}/configure" AND "${SRC_DIR}/configure.ac") # remove configure; rerun autoconf
         if(NOT VCPKG_MAINTAINER_SKIP_AUTOCONFIG) # If fixing bugs skipping autoconfig saves a lot of time
             set(REQUIRES_AUTOCONFIG TRUE)
             file(REMOVE "${SRC_DIR}/configure") # remove possible autodated configure scripts
             set(_csc_AUTOCONFIG ON)
         endif()
-    elseif(EXISTS "${SRC_DIR}/configure" AND NOT _csc_SKIP_CONFIGURE) # run normally; no autoconf or autogen required
+    elseif(EXISTS "${SRC_DIR}/configure") # run normally; no autoconf or autgen required
     elseif(EXISTS "${SRC_DIR}/configure.ac") # Run autoconfig
         set(REQUIRES_AUTOCONFIG TRUE)
         set(_csc_AUTOCONFIG ON)

@JackBoosY
Copy link
Contributor Author

@BillyONeal If the source contains configure or configure.ac, we must use it.

@BillyONeal
Copy link
Member

Even if the portfile says SKIP_CONFIGURE? If it says SKIP_CONFIGURE but we configure anyway that's..... odd.

@JackBoosY
Copy link
Contributor Author

@BillyONeal Yep, configure is always paired with makefile.in. So when using SKIP_CONFIGURE and exists configure, the command make cannot be used directly.

if exists configure/configure.ac -> use ./configure
else if SKIP_CONFIGURE -> run make
else -> error

@BillyONeal
Copy link
Member

when using SKIP_CONFIGURE and exists configure, the command make cannot be used directly.

Isn't that up to portfile.cmake though? If there's an option named SKIP_CONFIGURE, and a configure happens, that's extremely surprising. If a port wants configure to be used, then why would they supply that option?

if exists configure/configure.ac -> use ./configure
else if SKIP_CONFIGURE -> run make
else -> error

That's the existing behavior, but I don't necessarily think that it's correct behavior.

@JackBoosY
Copy link
Contributor Author

@Neumann-A what's your suggestion?

@Neumann-A
Copy link
Contributor

The following should be true:
If configure.ac exists and AUTOCONF is given run autoconf.
If configure.ac and configure exists and no AUTOCONF is given rerun autoconf anyway. (due to --editable builds)
If you want to run autoconf highly depends with which version of autoconf the configure script + libtool has been created. vcpkg needs a rather recent one so that cl.exe can run correctly. So If you don't want to run autoconf just delete the configure.ac before vcpkg_configure_make

If SKIP_CONFIGURE exists and configure and/or configure.ac exists -> be highly suspicious. It is expected that the portfile deletes the files manually and documents why they are not necessary. In general. Why are you even using SKIP_CONFIGURE with vcpkg_configure_make? Maybe SKIP_CONFIGURE should just be removed.

@JackBoosY
Copy link
Contributor Author

@BillyONeal @Neumann-A
I think these options (AUTO_CONFIG/SKIP_CONFIGURE etc.) need to be explicitly declared by the maintainer, otherwise the behavior in vcpkg_configure_make may violate the maintainer’s intention.

@ianichitei
Copy link
Contributor

ianichitei commented Nov 10, 2020

Why are you even using SKIP_CONFIGURE with vcpkg_configure_make? Maybe SKIP_CONFIGURE should just be removed.

Seeing how this was started by a issue opened by me and how no existing port seems to use SKIP_CONFIGURE I think I should expand a bit on my situation here and why I ended up using SKIP_CONFIGURE.

My opinion may be based on wrong assumptions or a lack of understanding of how vcpkg works, but I hope it can help you in making a decision.

I have a project that uses make in a pretty standard way and I want to write a port for it. I provided https://github.com/ianichitei/test as an example.

I ended up on the vcpkg_build_make docs, which state:

This command should be preceeded by a call to vcpkg_configure_make()

This is where things started to become unclear. It is not implied that I should use it, but it also seems like it is needed. It is not made entirely clear what happens if I don't set SOURCE_PATH , or how to make use of NO_DEBUG if I don't use vcpkg_configure_make.

However, I tried using directly vcpkg_build_make, but that produced various errors like:

CMake Error at scripts/cmake/vcpkg_execute_build_process.cmake:50 (file):
  file failed to open for reading (No such file or directory):

    /mnt/c/work/public/2/vcpkg/buildtrees/test/build-x64-linux-dbg-out.log

Trying to debug the issue I manually created that file and tried again. I got a similar error for build-x64-linux-dbg-err.log. After I manually created that file I still got errors:

$ ./vcpkg install test
Computing installation plan...
The following packages will be built and installed:
    test[core]:x64-linux
Detecting compiler hash for triplet x64-linux...
Starting package 1/1: test:x64-linux
Building package test[core]:x64-linux...
Could not locate cached archive: /home/ianichitei-lx/.cache/vcpkg/archives/34/3426704dc566cb043e5e7d12835eb806e5a88651.zip
-- Using cached /mnt/c/work/public/2/vcpkg/downloads/ianichitei-test-c2b1eafd43d66099fdfd27a634d75020a216ca7b.tar.gz
-- Cleaning sources at /mnt/c/work/public/2/vcpkg/buildtrees/test/src/20a216ca7b-d186164bf6.clean. Use --editable to skip cleaning for the packages you specify.
-- Extracting source /mnt/c/work/public/2/vcpkg/downloads/ianichitei-test-c2b1eafd43d66099fdfd27a634d75020a216ca7b.tar.gz
-- Using source at /mnt/c/work/public/2/vcpkg/buildtrees/test/src/20a216ca7b-d186164bf6.clean
-- SOURCE_PATH: /mnt/c/work/public/2/vcpkg/buildtrees/test/src/20a216ca7b-d186164bf6.clean
-- Building x64-linux-dbg
CMake Error at scripts/cmake/vcpkg_execute_build_process.cmake:136 (message):
    Command failed: /usr/bin/make V=1 -j 13 -f Makefile test
    Working Directory: /mnt/c/work/public/2/vcpkg/buildtrees/test/x64-linux-dbg
    See logs for more information:

Call Stack (most recent call first):
  scripts/cmake/vcpkg_build_make.cmake:200 (vcpkg_execute_build_process)
  ports/test/portfile.cmake:10 (vcpkg_build_make)
  scripts/ports.cmake:79 (include)


Error: Building package test:x64-linux failed with: BUILD_FAILED
Please ensure you're using the latest portfiles with `.\vcpkg update`, then
submit an issue at https://github.com/Microsoft/vcpkg/issues including:
  Package: test:x64-linux
  Vcpkg version: 2020.06.15-unknownhash

Additionally, attach any relevant sections from the log files above.

So it turns out that the working directory is not created. I managed to fix this with:

    vcpkg_configure_make(
        SOURCE_PATH ${SOURCE_PATH}
        COPY_SOURCE
        SKIP_CONFIGURE
    )

Using only COPY_SOURCE seems to make vcpkg copy my home directory, while using only SOURCE_PATH ${SOURCE_PATH} still leaves the working directory empty. Seeing this behavior I considered that vcpkg_configure_make is mandatory and that SKIP_CONFIGURE is an escape hatch for people like me.

The above examples are with an older vcpkg version on which SKIP_CONFIGURE worked for my case (458c20e).
I'm using WSL, I'm not entirely sure if this issue is WSL specific or not.

@Neumann-A
Copy link
Contributor

So you should be using only vcpkg_build_make and manually copy the source dir to the working dir. There is nothing to configure since your build only consists of makefiles so using vcpkg_configure_make is wrong.

@ianichitei
Copy link
Contributor

So you should be using only vcpkg_build_make and manually copy the source dir to the working dir. There is nothing to configure since your build only consists of makefiles so using vcpkg_configure_make is wrong.

Can you point me to the part of the docs that talks about this? The current documentation doesn't really explain the proper way of doing this. Copying the files manually seemed like a hack to me so I tried to not do that.

@strega-nil
Copy link
Contributor

strega-nil commented Nov 12, 2020

Closing this as "we should just remove SKIP_CONFIGURE".

@JackBoosY could you do that?

@strega-nil strega-nil closed this Nov 12, 2020
@JackBoosY
Copy link
Contributor Author

JackBoosY commented Nov 13, 2020

To be honest, using only vcpkg_build_make and manually creating the binary path(such as x64-linux-dbg and x64-linux-rel) in portfile.cmake is a very ugly thing. This destroys the encapsulation of vcpkg_*_make. And sometimes we need to copy the source code to the binary path.

@ianichitei
Copy link
Contributor

To be honest, using only vcpkg_build_make and manually creating the binary path(such as x64-linux-dbg and x64-linux-rel) in portfile.cmake is a very ugly thing. This destroys the encapsulation of vcpkg_*_make. And sometimes we need to copy the source code to the binary path.

So is doing a manual copy the only solution? It looks like a hack. vcpkg is already able to do this copy so maybe there should be a way of doing COPY_SOURCE without vcpkg_configure_make? I'd also suggest some improvements to the documentation in order to avoid future confusions when a newcomer like me reads it, if possible.

@JackBoosY
Copy link
Contributor Author

@ianichitei I think we need more discuss here.

@uvr-jra
Copy link

uvr-jra commented Jan 6, 2021

So you should be using only vcpkg_build_make and manually copy the source dir to the working dir. There is nothing to configure since your build only consists of makefiles so using vcpkg_configure_make is wrong.

Hi,

I am currently facing the same issue with a GitHub project providing a Makefile but no configure script.

Please, could you provide more details about the commands to call in the portfile to copy the source in the working dir before calling vcpkg_build_make() ?

Thanks a lot,

@JackBoosY
Copy link
Contributor Author

@uvr-jra I have another solution to solve that, let's wait for my next PR.

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:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

vcpkg_configure_make seems to ignore SKIP_CONFIGURE
7 participants