Skip to content

Commit 2c25a64

Browse files
committed
Review: a file added to a subproject or an overlay re-runs the installation; uic, rcc and lrelease refuse two inputs with one output name; the Widgets fixture runs on macOS
Found by a review of the branch against mcpp's SPEC-007. The added-file criterion fails with the watch removed and passes with it.
1 parent 5bff85b commit 2c25a64

7 files changed

Lines changed: 56 additions & 14 deletions

File tree

‎.github/scripts/check-deps-and-qt.sh‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ ROOT=$(cd "$(dirname "$0")/../.." && pwd)
1515
fail() { echo "FAIL: $*"; exit 1; }
1616

1717
is_windows() { case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) return 0 ;; *) return 1 ;; esac; }
18+
is_macos() { [ "$(uname -s)" = Darwin ]; }
1819

1920
# The stamp an installation action leaves: the file mcpp writes when a `check`
2021
# action's command succeeds. Its modification time is the criterion for "the
@@ -104,6 +105,17 @@ cmake_consumer() {
104105
# And not again: the engine moves the stamp past the input that changed
105106
# (mcpp's SPEC-007 R3.5).
106107
assert_not_rerun "$(stamp_of deps-cmake)"
108+
109+
# A file ADDED to the subproject is an input too: the build program watches
110+
# the tree, so the next plan names it and the installation runs again.
111+
trap 'rm -f "$ROOT/tests/cmake-consumer/greet/added.txt"' RETURN
112+
touch -r "$(stamp_of deps-cmake)" target/ci/before-added-file
113+
sleep 1
114+
echo added > greet/added.txt
115+
"$MCPP" build > target/ci/added-build.log 2>&1 || { cat target/ci/added-build.log; fail "the build after adding a file failed"; }
116+
[ -n "$(find "$(stamp_of deps-cmake)" -newer target/ci/before-added-file)" ] ||
117+
fail "a file added to the subproject did not re-run its installation"
118+
echo "ok: a file added to the subproject re-ran its installation"
107119
}
108120

109121
qt_consumer() {
@@ -124,6 +136,10 @@ qt_widgets_consumer() {
124136
QT_QPA_PLATFORM=offscreen "$MCPP" run | tee run.log
125137
grep -q "^qt-widgets-consumer: platform offscreen, label 'made by uic'$" run.log ||
126138
fail "the platform plugin or the uic form did not reach the program"
139+
if is_macos; then
140+
find target -path '*/bin/platforms/libqoffscreen.dylib' | grep -q . ||
141+
fail "no platforms/libqoffscreen.dylib was deployed beside the program"
142+
fi
127143
if is_windows; then
128144
QT_QPA_PLATFORM=offscreen run_directly qt-widgets-consumer | tee direct.log
129145
grep -q "^qt-widgets-consumer: platform offscreen" direct.log ||

‎.github/workflows/ci.yml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2334,6 +2334,8 @@ jobs:
23342334
- name: rules-qt runs moc, rcc and lrelease, with the SDK from xim:qt
23352335
run: bash .github/scripts/check-deps-and-qt.sh qt-consumer
23362336

2337+
# Windows and macOS: QtGui's dependencies are the system's there. On Linux
2338+
# QtGui loads libdbus, which the ecosystem does not publish, so the Linux
2339+
# job runs the console fixture only.
23372340
- name: rules-qt builds a Widgets program with a .ui form, and it runs offscreen
2338-
if: runner.os == 'Windows'
23392341
run: bash .github/scripts/check-deps-and-qt.sh qt-widgets-consumer

‎deps/cmake.cppm‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ inline prefix use(const options& opt) {
146146
for (auto const& x : opt.cache_args) a.arg(x.c_str());
147147
a.input(tool.c_str());
148148
for (auto const& f : mcpp::deps::files_under(source)) a.input(f.c_str());
149+
mcpp::deps::watch_tree(source);
149150
a.output(stamp.c_str());
150151
a.output_dir(p.root.c_str());
151152
a.submit();

‎deps/deps.cppm‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,16 @@ inline std::vector<std::string> files_under(const std::filesystem::path& dir) {
123123
return out;
124124
}
125125

126+
// Re-runs the build program when the SET of files under `dir` changes, so a
127+
// file added to a subproject or an overlay becomes an input of the action the
128+
// next plan declares; `files_under` names the files that exist now. The
129+
// pattern is relative to the package root, as `rerun_if_changed_glob` takes it.
130+
inline void watch_tree(const std::filesystem::path& dir) {
131+
const auto rel = dir.lexically_relative(std::filesystem::path(mcpp::manifest_dir()));
132+
const std::string pattern = (rel.empty() ? std::string(".") : rel.generic_string()) + "/**";
133+
mcpp::rerun_if_changed_glob(pattern.c_str());
134+
}
135+
126136
// The file a library name denotes under `lib_dir`. A name that already carries
127137
// an extension is a file name and is taken as written; otherwise the target's
128138
// convention decides: `<name>.lib` on Windows (an import library and a static

‎deps/vcpkg.cppm‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,10 +256,14 @@ inline prefix use(const options& opt = {}) {
256256
if (fs::is_regular_file(configFile, ec)) a.input(mcpp::deps::generic(configFile).c_str());
257257
// An overlay's files are inputs: a changed patch or triplet is a
258258
// different installation.
259-
for (auto const& d : overlayTriplets)
259+
for (auto const& d : overlayTriplets) {
260260
for (auto const& f : mcpp::deps::files_under(d)) a.input(f.c_str());
261-
for (auto const& d : overlayPorts)
261+
mcpp::deps::watch_tree(d);
262+
}
263+
for (auto const& d : overlayPorts) {
262264
for (auto const& f : mcpp::deps::files_under(d)) a.input(f.c_str());
265+
mcpp::deps::watch_tree(d);
266+
}
263267
a.output(stamp.c_str());
264268
a.output_dir(p.root.c_str());
265269
a.submit();

‎rules/qt.cppm‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -425,15 +425,21 @@ inline bool compile(options opt = {}) {
425425
headers.clear();
426426
inlineMoc.clear();
427427
}
428-
std::map<std::string, fs::path> mocNames;
428+
// EVERY GENERATED NAME HAS ONE SOURCE. Each generator names its output
429+
// after the input's stem, in one directory, so two inputs with one stem in
430+
// different directories would be two actions writing one file; refused
431+
// naming both, for moc, uic, rcc and lrelease alike.
432+
std::map<std::string, fs::path> claimed;
433+
auto claim = [&](const std::string& outName, const fs::path& in) -> bool {
434+
auto [it, fresh] = claimed.try_emplace(outName, in);
435+
if (fresh) return true;
436+
std::cerr << std::format("{}: two files produce `{}`: {} and {}. Generated files are "
437+
"named after the input's stem; rename one.\n",
438+
who, outName, generic(it->second), generic(in));
439+
return false;
440+
};
429441
auto mocOne = [&](const fs::path& in, const std::string& outName) -> bool {
430-
auto [it, fresh] = mocNames.try_emplace(outName, in);
431-
if (!fresh) {
432-
std::cerr << std::format("{}: two files produce `{}`: {} and {}. moc outputs are named "
433-
"after the file's stem; rename one.\n",
434-
who, outName, generic(it->second), generic(in));
435-
return false;
436-
}
442+
if (!claim(outName, in)) return false;
437443
const std::string out = generic(gen / outName);
438444
const std::string dep = out + ".d";
439445
const std::string src = generic(in);
@@ -464,6 +470,7 @@ inline bool compile(options opt = {}) {
464470
forms.clear();
465471
}
466472
for (auto const& f : forms) {
473+
if (!claim("ui_" + fs::path(f).stem().string() + ".h", detail::absolute_from_root(f))) return false;
467474
const std::string in = generic(detail::absolute_from_root(f));
468475
const std::string out = generic(gen / ("ui_" + fs::path(f).stem().string() + ".h"));
469476
const std::string id = "qt:uic:" + fs::path(f).stem().string();
@@ -490,6 +497,7 @@ inline bool compile(options opt = {}) {
490497
for (auto const& r : resources) {
491498
const fs::path qrc = detail::absolute_from_root(r);
492499
const std::string stem = qrc.stem().string();
500+
if (!claim("qrc_" + stem + ".cpp", qrc)) return false;
493501
const std::string in = generic(qrc);
494502
const std::string out = generic(gen / ("qrc_" + stem + ".cpp"));
495503
const std::string id = "qt:rcc:" + stem;
@@ -532,6 +540,7 @@ inline bool compile(options opt = {}) {
532540
for (auto const& t : ts) {
533541
const fs::path file = detail::absolute_from_root(t);
534542
const std::string stem = file.stem().string();
543+
if (!claim(stem + ".qm", file)) return false;
535544
const std::string in = generic(file);
536545
const fs::path qmDir = opt.i18n.out_dir.empty() ? gen / "translations"
537546
: detail::absolute_from_root(opt.i18n.out_dir);

‎tests/qt-widgets-consumer/mcpp.toml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ plugins = { path = "../..", features = ["rules-qt-xim"], host-module = true }
2525
[build]
2626
sources = ["src/*.cpp", "ui/*.ui"]
2727

28-
# Built and run on Windows by CI; on Linux QtGui needs libdbus, which the
29-
# ecosystem does not publish. The Linux runtime contract below states what a
30-
# Linux build of it will need once it does.
28+
# Built and run on Windows and macOS by CI; on Linux QtGui needs libdbus, which
29+
# the ecosystem does not publish. The Linux runtime contract below is what a
30+
# Linux build of it states.
3131
#
3232
# Qt's Linux libraries load the shared libstdc++ (`libstdc++.so.6`), so a
3333
# program that links them uses the toolchain's shared C++ runtime rather than

0 commit comments

Comments
 (0)