search

cisco / ChezScheme

Chez Scheme
Apache License 2.0
6.89k stars 983 forks source link

adjust `configure ZUO=<command>` support #816

Closed mflatt closed 3 months ago

mflatt commented 3 months ago

Continuing from b8838c3280, 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 commented 3 months ago

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 https://github.com/cisco/ChezScheme/pull/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 commented 3 months ago

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 commented 3 months ago

LGTM, then!

LiberalArtist commented 3 months ago

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".