Makefile cleanups #5017
Merged
Makefile cleanups #5017
Conversation
This file contains 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
openshift-ci-robot
added
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
I'm not sure, looking... |
|
...okay, the only user (I'm aware of) for that target is
So the file-name targets are not needed AFAICT, so long as the above targets function (and spit out the expected files). Great detective work BTW 😀 |
|
Thanks, @cevich! I updated the PR. The Makefile is now fast again. git-bisect to the rescue :) |
Move the systemd-buildtag check into the `bin/podman` target. No need to execute the check for all invocations of the Makefile. Signed-off-by: Valentin Rothberg <[email protected]>
Signed-off-by: Valentin Rothberg <[email protected]>
Add a .PHONY line over each target instead of mixing this notation with a separate but incomplete single list. Signed-off-by: Valentin Rothberg <[email protected]>
We don't set it, so there's no need to keep it. Signed-off-by: Valentin Rothberg <[email protected]>
Just echo the message instead of warning to not impact the exit code. Signed-off-by: Valentin Rothberg <[email protected]>
Speed up the Makefile by removing variable references from the release targets. Now, the variables will only be (lazily) evaluated when they are actually needed and not for each invocation of the Makefile which has it down considerably. Fixes: containers#4829 Signed-off-by: Valentin Rothberg <[email protected]>
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
LGTM |
|
/lgtm |
edsantiago
added a commit
to edsantiago/libpod
that referenced
this pull request
Feb 3, 2021
Backstory: every time you run 'make podman' or even just 'make', you get a full recompile. This is sub-ideal. Cause: I don't really know. It looks complicated. containers#5017 introduced a .PHONY for bin/podman, for reasons not explained in the PR. Then, much later, containers#5880 well- intentionedly but improperly tweaked the 'find' command used in defining SOURCES, adding a -prune but without the corresponding and required -print. Let's just say, it was an unfortunate cascade of events. This PR fixes the SOURCES definition and removes the highly-undesired .PHONY from podman & podman-remote, making it so you can type 'make' and, oh joy, not build anything if it's current. The way 'make' is supposed to work. Why fix this now? Because my PR (containers#9209) was failing in CI, in the Validate step: Can't exec "./bin/podman": No such file or directory at hack/xref-helpmsgs-manpages line 223. It failed even on Re-run, and only passed once I force-pushed the PR (with no changes, just a new commit SHA). I have no idea why bin/podman wasn't built, and I have zero interest in pursuing that right now, but the proper solution is to add bin/podman as a Makefile dependency for that particular test. So done. While I'm at it, fix what is pretty clearly a typo in a .PHONY And, finally, fix a go-md2man warning introduced in containers#9189 [NO TESTS NEEDED] Signed-off-by: Ed Santiago <[email protected]>
github-actions
bot
added
the
locked - please file new issue/PR
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
A set of cleanups for the Makefile. I'm ultimately aiming at making it faster (and fix #4829) since it got very slow over time.
I bisected the performance and found that the release targets are responsible:
https://github.com/containers/libpod/blob/master/Makefile#L364-L372
@cevich, are
podman-v$(RELEASE_NUMBER).tar.gz:and the remote counterpart used in the CI? GeneratingRELEASE_NUMBERfor each invocation of the Makefile is burning most of the time. Commenting the two targets renders amake tab tabcompletion fast again. If needed, any chance we could not embed that in the target name?