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

[qtbase] no absolute paths #31762

Merged
merged 1 commit into from
Jun 13, 2023
Merged

Conversation

autoantwort
Copy link
Contributor

Otherwise the build fails with:

-- Performing post-build validation
warning: There should be no absolute paths, such as the following, in an installed package:
    /Users/leanderSchulten/git_projekte/vcpkg/packages/qtbase_arm64-osx
    /Users/leanderSchulten/git_projekte/vcpkg/vcpkg_installed
    /Users/leanderSchulten/git_projekte/vcpkg/buildtrees/qtbase
    /Users/leanderSchulten/git_projekte/vcpkg/downloads
Absolute paths were found in the following files:
    /Users/leanderSchulten/git_projekte/vcpkg/packages/qtbase_arm64-osx/tools/Qt6/bin/qt-cmake-private
    /Users/leanderSchulten/git_projekte/vcpkg/packages/qtbase_arm64-osx/tools/Qt6/bin/debug/qt-cmake-private
    /Users/leanderSchulten/git_projekte/vcpkg/packages/qtbase_arm64-osx/tools/Qt6/bin/debug/qt-cmake
    /Users/leanderSchulten/git_projekte/vcpkg/packages/qtbase_arm64-osx/tools/Qt6/bin/qt-cmake

And the file content looks like:

#!/bin/sh

# The directory of this script is the expanded absolute path of the "$qt_prefix/bin" directory.
script_dir_path=`dirname $0`
script_dir_path=`(cd "$script_dir_path"; /bin/pwd)`

# Try to use original cmake, otherwise to make it relocatable, use any cmake found in PATH.
original_cmake_path="/Users/leanderSchulten/git_projekte/vcpkg/downloads/tools/cmake-3.25.1-osx/cmake-3.25.1-macos-universal/CMake.app/Contents/bin/cmake"
cmake_path=$original_cmake_path
if ! test -f "$cmake_path"; then
    cmake_path="cmake"
fi

toolchain_path="$script_dir_path/../../../share/Qt6/qt.toolchain.cmake"

# Find the qt toolchain relative to the absolute bin dir path where the script is located.
exec "$cmake_path" -DCMAKE_TOOLCHAIN_FILE="$toolchain_path" -G"Ninja" "$@"

@BillyONeal
Copy link
Member

Notably, the failure doesn't happen with the current vcpkg-tool version bootstrap fetches, but it does happen with the soon-to-be-merged copy.

In #31723 I resolved it in favor of turning off the check because the ultimate goal, relocatablity, is maintained without any edits, as the absolute path is just an attempted search location.

However, I think this resolution is reasonable too; I don't have strong opinions on it.

See also, kf5i18n with a very similar 'absolute path then fallback' behavior for Python.

@autoantwort
Copy link
Contributor Author

The problem is that if we disable the check it will also not report the cases that are preventing relocatablity in the future.

@BillyONeal
Copy link
Member

The problem is that if we disable the check it will also not report the cases that are preventing relocatablity in the future.

Yeah, I understand, that's why I'm not opposed to this. Just wanted to make sure this was considered.

@Adela0814 Adela0814 added the category:port-bug The issue is with a library, which is something the port should already support label Jun 2, 2023
@Adela0814
Copy link
Contributor

CI failed, please find the details from https://dev.azure.com/vcpkg/public/_build/results?buildId=90161&view=artifacts&pathAsName=false&type=publishedArtifacts.
CI failures:

CMake Error at ports/qtbase/portfile.cmake:475 (file):
  file failed to open for reading (No such file or directory):

    D:/packages/qtbase_x64-windows/tools/Qt6/bin/qt-cmake
Call Stack (most recent call first):
  ports/qtbase/portfile.cmake:481 (remove_original_cmake_path)
  scripts/ports.cmake:147 (include)

@Adela0814 Adela0814 marked this pull request as draft June 2, 2023 03:07
@autoantwort autoantwort force-pushed the qtbase-no-absolute branch 3 times, most recently from 66a3538 to feb244b Compare June 6, 2023 13:58
@autoantwort
Copy link
Contributor Author

@Adela0814 I have fixed the ci errors

@autoantwort autoantwort marked this pull request as ready for review June 13, 2023 17:21
@BillyONeal BillyONeal merged commit d9f5411 into microsoft:master Jun 13, 2023
15 checks passed
@BillyONeal
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants