Unbreak Makefile #707
Merged
Unbreak Makefile #707
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
Drop unneeded variables, reorganize the precedence of .PHONY in accordance with standard style (this has no reason to be one of the first things people see).
Previously, all targets were .PHONY targets, which meant that on every invocation of `make`, all targets were unconditionally rebuilt. For example, running `make && make install` would build gopass, then rebuild it in order to build the completions. The solution is to properly use file-based dependencies. The gopass target is used to generate the gopass binary, and properly depends on all its source files. The completion files are generated using a Makefile pattern, and regenerated whenever the gopass binary is. Various other commands are modified to depend on the binary instead of the build target (which is now an alias to build the gopass target). While I am at it, the default `make` target (that is to say, the first listed rule) should *not* be the one that runs all of the project's own internal unit testing and coverage commands. So, make the default `make all` target build the project and generate completion files, and use the `make travis` target for, well, making travis-related things.
Codecov Report
@@ Coverage Diff @@
## master #707 +/- ##
=======================================
Coverage 65.48% 65.48%
=======================================
Files 152 152
Lines 8690 8690
=======================================
Hits 5690 5690
Misses 2328 2328
Partials 672 672Continue to review full report at Codecov.
|
|
Thank you very much for this PR. While I disagree with the title (technically the Makefile is not broken), the changes make perfect sense! |
|
Eh, probably could have reworded that title, but it was 2AM (3AM if you count daylight savings time issues), and I was fighting with actually testing my changes because 1) it doesn't seem to pick up vendor/ without workarounds grabbed from the Arch build script, and 2) the binary instantly segfaults for reasons that eventually turned out to be "using a pie-enabled go compiler". |
|
No worries at all, I can relate. Just wanted to document the fact that the Makefile wasn't completely broken before ;) |
kpitt
pushed a commit
to kpitt/gopass
that referenced
this pull request
Jul 21, 2022
* Makefile: fix pointless repetition of a custom command Drop unneeded variables, reorganize the precedence of .PHONY in accordance with standard style (this has no reason to be one of the first things people see). * Makefile: fix completely broken target ordering. Previously, all targets were .PHONY targets, which meant that on every invocation of `make`, all targets were unconditionally rebuilt. For example, running `make && make install` would build gopass, then rebuild it in order to build the completions. The solution is to properly use file-based dependencies. The gopass target is used to generate the gopass binary, and properly depends on all its source files. The completion files are generated using a Makefile pattern, and regenerated whenever the gopass binary is. Various other commands are modified to depend on the binary instead of the build target (which is now an alias to build the gopass target). While I am at it, the default `make` target (that is to say, the first listed rule) should *not* be the one that runs all of the project's own internal unit testing and coverage commands. So, make the default `make all` target build the project and generate completion files, and use the `make travis` target for, well, making travis-related things.
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.
This fixes an issue where distro builds violated an expectation that software should be built when building and installed while installing. Instead, gopass was repeatedly rebuilt during the packaging phase, which is not supposed to modify the release artifacts.
This is probably not very enjoyable even without it violating distro packaging guidelines...
By making proper use of targets, we can also ensure that simply running
makewill rebuild the binary iff it needs to be rebuilt, so this serves the additional utility of avoiding extra work and making it obvious if changes to the source files have been made since the last rebuild./cc @Foxboron