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

Restore printing full paths to files containing absolute paths. #1455

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions azure-pipelines/end-to-end-tests-dir/post-build-checks.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -948,9 +948,7 @@ note: $($buildtreesRoot)
"@
# downloads directory here
$expectedFooter = @"
note: Absolute paths were found in the following files
$($packagesRoot)$($NativeSlash)vcpkg-policy-absolute-paths_$($Triplet): note: the files are relative to `${CURRENT_PACKAGES_DIR} here
note: include/vcpkg-policy-absolute-paths.h
$($packagesRoot)$($NativeSlash)vcpkg-policy-absolute-paths_$($Triplet)$($NativeSlash)include$($NativeSlash)vcpkg-policy-absolute-paths.h: note: Absolute paths found here
"@

foreach ($bad_dir in @('build-dir', 'downloads', 'installed-root', 'package-dir')) {
Expand All @@ -966,11 +964,9 @@ foreach ($bad_dir in @('build-dir', 'downloads', 'installed-root', 'package-dir'

$expectedFooter = @"
$($PortfilePath): note: Adding a call to ``vcpkg_fixup_pkgconfig()`` may fix absolute paths in .pc files
note: Absolute paths were found in the following files
$($packagesRoot)$($NativeSlash)vcpkg-policy-absolute-paths_$($Triplet): note: the files are relative to `${CURRENT_PACKAGES_DIR} here
note: include/vcpkg-policy-absolute-paths.h
Copy link
Member Author

@BillyONeal BillyONeal Jul 12, 2024

Choose a reason for hiding this comment

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

I am a bit sad to lose the form that a user would say in portfile.cmake in order to edit one of these files but making the paths clickable is probably worth it anyway... should we keep printing ${CURRENT_PACKAGES_DIR} here to make it easier for the user to figure that out?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can see arguments either way... though doesn't this apply to most post-lint checks? Maybe it should be centralized (printed once at the beginning or end of the post-lint checks if any fired)

note: share/pkgconfig/vcpkg-policy-absolute-paths.pc
note: tools/vcpkg-policy-absolute-paths/bin/port-config.sh
$($packagesRoot)$($NativeSlash)vcpkg-policy-absolute-paths_$($Triplet)$($NativeSlash)include$($NativeSlash)vcpkg-policy-absolute-paths.h: note: Absolute paths found here
$($packagesRoot)$($NativeSlash)vcpkg-policy-absolute-paths_$($Triplet)$($NativeSlash)share$($NativeSlash)pkgconfig$($NativeSlash)vcpkg-policy-absolute-paths.pc: note: Absolute paths found here
$($packagesRoot)$($NativeSlash)vcpkg-policy-absolute-paths_$($Triplet)$($NativeSlash)tools$($NativeSlash)vcpkg-policy-absolute-paths$($NativeSlash)bin$($NativeSlash)port-config.sh: note: Absolute paths found here
"@

foreach ($bad_dir in @('build-dir', 'downloads', 'installed-root', 'package-dir')) {
Expand Down
2 changes: 1 addition & 1 deletion include/vcpkg/base/message-data.inc.h
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,7 @@ DECLARE_MESSAGE(FilesContainAbsolutePath1,
"followed by a list of found files.",
"There should be no absolute paths, such as the following, in an installed package. To suppress this "
"message, add set(VCPKG_POLICY_SKIP_ABSOLUTE_PATHS_CHECK enabled)")
DECLARE_MESSAGE(FilesContainAbsolutePath2, (), "", "Absolute paths were found in the following files")
DECLARE_MESSAGE(FilesContainAbsolutePath2, (), "", "Absolute paths found here")
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
DECLARE_MESSAGE(FilesContainAbsolutePath2, (), "", "Absolute paths found here")
DECLARE_MESSAGE(FilesContainAbsolutePath2, (), "", "absolute paths found here")

DECLARE_MESSAGE(FilesContainAbsolutePathPkgconfigNote,
(),
"",
Expand Down
2 changes: 1 addition & 1 deletion locales/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@
"_FileSeekFailed.comment": "An example of {path} is /foo/bar. An example of {byte_offset} is 42.",
"FilesContainAbsolutePath1": "There should be no absolute paths, such as the following, in an installed package. To suppress this message, add set(VCPKG_POLICY_SKIP_ABSOLUTE_PATHS_CHECK enabled)",
"_FilesContainAbsolutePath1.comment": "This message is printed before a list of found absolute paths, followed by FilesContainAbsolutePath2, followed by a list of found files.",
"FilesContainAbsolutePath2": "Absolute paths were found in the following files",
"FilesContainAbsolutePath2": "Absolute paths found here",
"FilesContainAbsolutePathPkgconfigNote": "Adding a call to `vcpkg_fixup_pkgconfig()` may fix absolute paths in .pc files",
"FilesExported": "Files exported at: {path}",
"_FilesExported.comment": "An example of {path} is /foo/bar.",
Expand Down
23 changes: 16 additions & 7 deletions src/vcpkg/postbuildlint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1589,23 +1589,32 @@ namespace vcpkg
LocalizedString::from_raw(portfile_cmake)
.append_raw(": ")
.append_raw(WarningPrefix)
.append(msgFilesContainAbsolutePath1));
.append(msgFilesContainAbsolutePath1)
.append_raw('\n'));
for (auto&& absolute_path : prohibited_absolute_paths)
{
msg_sink.print(LocalizedString::from_raw("\n").append_raw(NotePrefix).append_raw(absolute_path));
msg_sink.print(LocalizedString::from_raw(NotePrefix).append_raw(absolute_path).append_raw('\n'));
}

if (any_pc_file_fails)
{
msg_sink.print(LocalizedString::from_raw("\n")
.append_raw(portfile_cmake)
msg_sink.print(LocalizedString::from_raw(portfile_cmake)
.append_raw(": ")
.append_raw(NotePrefix)
.append(msgFilesContainAbsolutePathPkgconfigNote));
.append(msgFilesContainAbsolutePathPkgconfigNote)
.append_raw('\n'));
}

for (auto&& failing_file : failing_files)
{
failing_file.make_preferred();
msg_sink.print(LocalizedString::from_raw(package_dir / failing_file)
.append_raw(": ")
.append_raw(NotePrefix)
.append(msgFilesContainAbsolutePath2)
.append_raw('\n'));
}

msg_sink.print(LocalizedString::from_raw("\n").append_raw(NotePrefix).append(msgFilesContainAbsolutePath2));
print_relative_paths(msg_sink, msgFilesRelativeToThePackageDirectoryHere, package_dir, failing_files);
return LintStatus::PROBLEM_DETECTED;
}

Expand Down
Loading