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
Makefile.mk: address minor issues #10000
Closed
Closed
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
2fa27fe
to
23b8325
Compare
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Nov 29, 2022
Auto-detection used either `command -v` (for unixy shells) or `where` (for Windows ones). The former is an internal command, and it fails to run from an MSYS2 shell via a non-MSYS2 GNU Make, because make executes it as-is via `CreateProcess()`. This also results in a warning. Prepending `$(SHELL) -c` would fix it, but would also need to enclose the rest of the command line in quotes, what makes this code overly complex for little benefit. Using `which` would also solve it, but this isn't the correct way to check for tool presence and some *nix systems don't have it. Resolve this by dropping auto-detection and expect the caller to set the `NROFF` variable to `nroff`/`groff` tool, in case someone wants to rebuild the manual and such tool is available. Closes curl#10000 [ci skip]
23b8325
to
1454b0e
Compare
vszakats
added a commit
to vszakats/curl
that referenced
this pull request
Nov 29, 2022
Auto-detection used either `command -v` (for unixy shells) or `where` (for Windows ones). The former is an internal command, and it fails to run from an MSYS2 shell via a non-MSYS2 GNU Make, because make executes it as-is via `CreateProcess()`. This also results in a warning. Prepending `$(SHELL) -c` would fix it, but would also need to enclose the rest of the command line in quotes, what makes this code overly complex for little benefit. Using `which` would also solve it, but this isn't the correct way to check for tool presence and some *nix systems don't have it. Resolve this by dropping auto-detection and expect the caller to set the `NROFF` variable to `nroff`/`groff` tool, in case someone wants to rebuild the manual and such tool is available. Closes curl#10000 [ci skip]
b7f513b
to
f65df23
Compare
Auto-detection used either `command -v` (for unixy shells) or `where` (for Windows ones). The former is an internal command, and it fails to run from an MSYS2 shell via a non-MSYS2 GNU Make, because make executes it as-is via `CreateProcess()`. This also results in a warning. Prepending `$(SHELL) -c` would fix it, but would also need to enclose the rest of the command line in quotes, what makes this code overly complex for little benefit. Using `which` would also solve it, but this isn't the correct way to check for tool presence and some *nix systems don't have it. Resolve this by dropping auto-detection and expect the caller to set the `NROFF` variable to `nroff`/`groff` tool, in case someone wants to rebuild the manual and such tool is available. Closes curl#10000 [ci skip]
f65df23
to
e4b8b0e
Compare
This silences a list of warnings when GNU Make is run with the option `--warn-undefined-variables`. Another benefit is that it is now easy to look up all user-customizable `Makefile.mk` variables by grepping for ` ?=` in the curl source tree.
e4b8b0e
to
3661078
Compare
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.
Fix
NROFF
auto-detection with certain shell/make-build combinations:When a non-MSYS2 GNU Make runs inside an MSYS2 shell, Make executes
the detection command as-is via
CreateProcess()
. It fails becausecommand
is ansh
built-in. Ensure to explicitly invoke the shell.Initialize user-customizable variables:
Silences a list of warnings when running GNU Make with the option
--warn-undefined-variables
. Another benefit is that it's now easyto look up all user-customizable
Makefile.mk
variables by greppingfor
?=
in the curl source tree.Suggested-by: Gisle Vanem
Ref: Makefile.mk: portable Makefile.m32 #9764 (comment)
Fix
MKDIR
invocation:Avoid a warning and potential issue in envs without forward-slash
support.
Closes #10000