-
Notifications
You must be signed in to change notification settings - Fork 299
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
[post build lint] Check for absolute paths #172
Changes from 55 commits
863231d
15778b6
9212d1b
326672e
cef8cb0
761dc17
7fd595a
7e9fdd0
f58d9ae
6840f39
d7f0f47
42c62d7
80704ee
2c1fb86
cd19401
1fd5a30
6154d36
47787c5
86072b7
ea1e189
10049d7
03d24cd
b51e0b1
5d19692
b48fd0f
81c991d
aa5264b
8b2cc5c
d58b64e
6db65d5
04e77f5
dce3e55
9df3a83
dec596d
20a0747
cc4cbc8
7b535b4
dfa58d3
3fd3ea2
024b814
a1b3aa1
6421db4
bdcb84c
2da94b4
535e47a
72476ba
646525c
9b0c5f8
2329d8d
771a5a1
1994010
2df175b
23c9e7d
0a5c013
028dd92
a2a42d5
4092884
22c3d0e
c1188c6
f8b77a2
0937195
bc23f1f
298a2f4
4fabb06
ed7e9c8
cc3f741
ccf6b23
5222349
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,8 @@ | |
#include <algorithm> | ||
#include <vector> | ||
|
||
#include "vcpkg/base/fwd/span.h" | ||
|
||
namespace vcpkg::Strings::details | ||
{ | ||
// first looks up to_string on `T` using ADL; then, if that isn't found, | ||
|
@@ -218,6 +220,8 @@ namespace vcpkg::Strings | |
|
||
Optional<StringView> find_at_most_one_enclosed(StringView input, StringView left_tag, StringView right_tag); | ||
|
||
bool contains_any_ignoring_c_comments(const std::string& source, View<StringView> to_find); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
||
bool equals(StringView a, StringView b); | ||
|
||
template<class T> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,12 @@ | ||
#include <vcpkg/base/cofffilereader.h> | ||
#include <vcpkg/base/files.h> | ||
#include <vcpkg/base/messages.h> | ||
#include <vcpkg/base/system.print.h> | ||
#include <vcpkg/base/system.process.h> | ||
#include <vcpkg/base/util.h> | ||
|
||
#include <vcpkg/build.h> | ||
#include <vcpkg/installedpaths.h> | ||
#include <vcpkg/packagespec.h> | ||
#include <vcpkg/postbuildlint.buildtype.h> | ||
#include <vcpkg/postbuildlint.h> | ||
|
@@ -1025,6 +1027,95 @@ namespace vcpkg::PostBuildLint | |
return LintStatus::SUCCESS; | ||
} | ||
|
||
static LintStatus check_no_absolute_paths_in(const Filesystem& fs, const Path& dir, Span<Path> absolute_paths) | ||
{ | ||
static constexpr StringLiteral extensions[] = {"h", "hpp", "hxx", "py", "sh", "cmake", "pc", "cfg", "conf"}; | ||
std::vector<std::pair<Path, std::string>> files_and_contents; | ||
autoantwort marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (auto& path : fs.get_regular_files_recursive(dir, IgnoreErrors{})) | ||
{ | ||
if (path.extension().empty()) | ||
{ | ||
auto contents = fs.read_contents(path, VCPKG_LINE_INFO); | ||
if (Strings::starts_with(contents, "#!")) | ||
{ | ||
files_and_contents.emplace_back(std::move(path), std::move(contents)); | ||
} | ||
} | ||
else if (Util::contains(extensions, path.extension().substr(1 /* ignore dot */))) | ||
{ | ||
auto contents = fs.read_contents(path, VCPKG_LINE_INFO); | ||
files_and_contents.emplace_back(std::move(path), std::move(contents)); | ||
} | ||
} | ||
std::vector<std::string> string_paths; | ||
for (const auto& path : absolute_paths) | ||
{ | ||
#if defined(_WIN32) | ||
auto path_preferred = path; | ||
path_preferred.make_preferred(); | ||
string_paths.push_back(path_preferred.generic_u8string()); | ||
string_paths.push_back(std::move(path).native()); | ||
autoantwort marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#else | ||
string_paths.push_back(path.native()); | ||
#endif | ||
} | ||
const auto stringview_paths = Util::fmap(string_paths, [](std::string& s) { return StringView(s); }); | ||
|
||
std::string result; | ||
for (const auto& path_and_contents : files_and_contents) | ||
{ | ||
const auto extension = path_and_contents.first.extension().substr(1 /* ignore dot */); | ||
const bool is_header = extension == "h" || extension == "hpp" || extension == "hxx"; | ||
vicroms marked this conversation as resolved.
Show resolved
Hide resolved
|
||
bool found_absolute = false; | ||
if (is_header) | ||
{ | ||
found_absolute = Util::any_of(string_paths, [&path_and_contents](const std::string& path) { | ||
StringView sv(path); | ||
bool contains = path_and_contents.second.find(path) != std::string::npos; | ||
// First do a cheap search and then a complicated one | ||
return contains && Strings::contains_any_ignoring_c_comments(path_and_contents.second, {&sv, 1}); | ||
autoantwort marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}); | ||
} | ||
else | ||
{ | ||
found_absolute = Util::any_of(string_paths, [&path_and_contents, extension](const std::string& path) { | ||
if (extension == "cfg" || extension == "conf") | ||
{ | ||
return Strings::contains(path_and_contents.second, path); | ||
} | ||
for (size_t offset = 0;;) | ||
{ | ||
const auto index = path_and_contents.second.find(path, offset); | ||
if (index == std::string::npos) return false; | ||
{ // .py, .sh, .cmake or .pc file | ||
const auto before = path_and_contents.second.find_last_of("\n#", index); | ||
if (before == std::string::npos) return true; | ||
if (path_and_contents.second[before] == '\n') return true; // not a comment | ||
} | ||
offset = index + path.size(); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part looks extractable like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does that |
||
}); | ||
} | ||
if (found_absolute) | ||
{ | ||
result += "\n "; | ||
result += path_and_contents.first.native(); | ||
} | ||
} | ||
|
||
if (result.empty()) | ||
{ | ||
return LintStatus::SUCCESS; | ||
} | ||
|
||
msg::print(Color::warning, | ||
msg::format(msgFilesContainAbsolutePath, | ||
msg::absolute_paths = Strings::join("', '", absolute_paths), | ||
msg::paths = result) | ||
.append_raw("\n\n")); | ||
return LintStatus::PROBLEM_DETECTED; | ||
} | ||
|
||
static void operator+=(size_t& left, const LintStatus& right) { left += static_cast<size_t>(right); } | ||
|
||
static size_t perform_all_checks_and_return_error_count(const PackageSpec& spec, | ||
|
@@ -1153,6 +1244,13 @@ namespace vcpkg::PostBuildLint | |
error_count += check_no_files_in_dir(fs, package_dir); | ||
error_count += check_no_files_in_dir(fs, package_dir / "debug"); | ||
error_count += check_pkgconfig_dir_only_in_lib_dir(fs, package_dir); | ||
if (!build_info.policies.is_enabled(BuildPolicy::SKIP_ABSOLUTE_PATHS_CHECK)) | ||
{ | ||
error_count += check_no_absolute_paths_in( | ||
fs, | ||
package_dir, | ||
std::vector<Path>{package_dir.native(), paths.installed().root().native(), paths.build_dir(spec)}); | ||
autoantwort marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
return error_count; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to tell the user about the policy now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the policy should only be used internally to that merging this PR does the break the world. Imho we should not tell the users how to ignore warnings that warns before broken packages.