Skip to content
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

adjust configure ZUO=<command> support #816

Merged
merged 1 commit into from
Mar 17, 2024
Merged

adjust configure ZUO=<command> support #816

merged 1 commit into from
Mar 17, 2024

Conversation

mflatt
Copy link
Contributor

@mflatt mflatt commented Mar 10, 2024

Continuing from b8838c3, adjust the generated makefile so the supplied is not a makefile dependency. That way, ZUO=zuo works if zuo is installed and the current build directory is not the source directory. (The zuo executable is a dependency in a real and relevant sense, but not in the sense of dependencies that we normally track in makefiles.)

Also adapt the makefile for the case that ZUO=... is not supplied and the build directory is not the source directory, in which case ZUO_LIB_PATH needs to be relative to the source directory.

Using make ZUO=zuo can also work, but in that case, bin/zuo is still built as a dependency. It's possible that some portable makefile magic could overcome that limitation, but it doesn't seem important.

CC: @LiberalArtist

@LiberalArtist
Copy link
Contributor

Continuing from b8838c3, adjust the generated makefile so the supplied <command> is not a makefile dependency. That way, ZUO=zuo works if zuo is installed and the current build directory is not the source directory.

Just to make sure I'm following correctly, without this patch, is the problem that ./configure ZUO=zuo still had $(ZUO) (expanding to zuo) as a prerequisite in the makefile rules, but had no recipe to build zuo, so it only worked if a file or directory named zuo happened to already exist in the build directory (as is the case when building in the source directory)? I know I tested both ./configure ZUO=zuo and out-of-source builds, but maybe I didn't test the right combination of configurations to see the problem.

Using make ZUO=zuo can also work, but in that case, bin/zuo is still built as a dependency. It's possible that some portable makefile magic could overcome that limitation, but it doesn't seem important.

Before #807, I had been using make ZUO=zuo, but, even so, I'm inclined to agree that this minor limitation isn't worth the complexity I expect would be needed to avoid it portably.

@mflatt
Copy link
Contributor Author

mflatt commented Mar 11, 2024

Just to make sure I'm following correctly, without this patch, is the problem that ./configure ZUO=zuo still had $(ZUO) (expanding to zuo) as a prerequisite in the makefile rules, but had no recipe to build zuo, so it only worked if a file or directory named zuo happened to already exist in the build directory (as is the case when building in the source directory)?

Yes, that's right.

@LiberalArtist
Copy link
Contributor

LGTM, then!

@LiberalArtist
Copy link
Contributor

Oh, trivially, in the commit message, you may want to put backticks around <command>: otherwise, where passed as Markdown, it's treated as an unsupported raw HTML tag, so it doesn't render, e.g. "so the supplied is not a makefile dependency".

Continuing from b8838c3, adjust the generated makefile so the
supplied `<command>` is not a makefile dependency. That way, `ZUO=zuo`
works if `zuo` is installed and the current build directory is not the
source directory. (The `zuo` executable is a dependency in a real and
relevant sense, but not in the sense of dependencies that we normally
track in makefiles.)

Also adapt the makefile for the case that `ZUO=...` is not supplied
and the build directory is not the source directory, in which case
`ZUO_LIB_PATH` needs to be relative to the source directory.

Using `make ZUO=zuo` can also work, but in that case, `bin/zuo` is
still built as a dependency. It's possible that some portable makefile
magic could overcome that limitation, but it doesn't seem important.
@mflatt mflatt merged commit d327968 into cisco:main Mar 17, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants