fix(scripts): make .PHONY in package.mk an actual declaration - #4
Open
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Open
fix(scripts): make .PHONY in package.mk an actual declaration#4Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Conversation
Line 2 used `.PHONY=`, which is a variable assignment. Make defined a variable that nothing reads, and none of the listed targets was phony. The special target needs a colon. Line 1 is `.DEFAULT_GOAL=help`, a real variable where `=` is correct, which is likely why this went unnoticed. Corrected the list to what this file actually defines. `suspend`, `resume`, `check` and `clean` were missing; `check` is the prerequisite that keeps `show`, `diff`, `apply` and `delete` rebuilding. Dropped `update` and `image`, which are defined neither here nor in any package Makefile: naming an undefined target in `.PHONY` creates it empty, so `make image` would report nothing to be done and exit 0 instead of failing with "No rule to make target". Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe package Makefile updates its ChangesPackage targets
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Line 2 of
scripts/package.mkis.PHONY=help show diff apply delete update image. That is a variable assignment, so make defines a variable called.PHONYthat nothing reads, and none of those targets is phony. Line 1 is.DEFAULT_GOAL=help, a real variable where=is correct, which is probably why this went unnoticed.I measured it both ways in a scratch checkout. With a file named
helpsitting next topackages/apps/minecraft-server/Makefile,make helpon current main printsmake: 'help' is up to date.and exits 0 without running the recipe. With the colon in place the recipe runs. Nothing in the repository is named after a target today, so this is latent rather than currently broken.I changed the list as well.
scripts/package.mkdefineshelp show apply diff suspend resume delete check clean, sosuspend,resume,checkandcleanwere missing.checkis the one that matters most, since it is the prerequisite that keepsshow,diff,applyanddeleterebuilding. Going the other way,updateandimageare not defined here at all, neither inscripts/package.mknor in any of the three package Makefiles. Naming an undefined target in.PHONYcreates it empty, somake imagewould print "Nothing to be done" and exit 0 instead of failing with "No rule to make target". Better to leave them out.One note on provenance: this file is a byte-for-byte copy of
hack/package.mkin cozystack/cozystack, where the same line is being fixed in cozystack/cozystack#3353. The two are identical today, so that change splits them. The target list over there is not automatically right here, and the one in this PR comes from what this repository's own Makefiles define.Summary by CodeRabbit
updateandimagebuild commands withsuspend,resume, andcheckcommands.