Bug report: some Makefile variables like ${V} are problematic
Closed this issue ยท 16 comments
I'm not filling out the usual form here because it's a bug with the Makefile and not something that a bug report log can help with. It's also not a single issue that can be identified but rather an issue with the concept that could break many rules.
The issue is the as I noted the Makefile variable (default value at least) ${V}
. The default is @:
which is a no-op action. But then it prevents the rest of the command from doing anything at all or so it would appear.
For instance doing make build_man
will not work as it stands (I'm about to commit a fix for this one in particular but it might be a problem elsewhere in the Makefile) because of the ${V}
variable in front of the make invocations. I fixed this part in commit 9f3b067 but the concept might need to be re-evaluated.
I couldn't figure out why I sometimes had the man/
subdirectory but then I remembered that I made the bug_report.sh
script have all debug options for Makefiles! This is why the make all
via the bug_report.sh
actually builds the man/
subdirectory but make all
outside of bug_report.sh
does not. Thus ironically doing make clobber all
would not create the man/ subdirectory but doing make clobber bug_report
would!
The fix in that case is to change it to ${Q}
and which will be committed shortly. But it might be that other Makefile rules need to be evaluated as far as whether or not they should use ${V}
.
I will update this issue with the commit in a moment.
For the build_man fix see commit 9f3b067.
Note the following point:
The solution in the case of build_man is to change the ${V} to ${Q} but
there might be other places this should be done: not sure but that's why
I opened the issue.
Either way it appears that having @: in a Makefile rule will prevent
anything beyond the : from being read as if it was a comment just like
the shell. Thus any line in the Makefiles that has that variable at the
front will not be executed further.
This means that echo statements or anything else that have this will also not run. I don't know what you want to do about that but it seems to be just like the shell e.g. doing:
$ : echo test
$ : make clobber
$
Notice how the commands don't do anything. The same appears to happen in the Makefiles though and this might be a problem at times.
Other variables could potentially cause this same problem: any of those that have a : in front of it. I know that the others currently don't have that but in comments it suggests that it simply disables the debug information with the @:
but in fact it disables the command entirely.
For instance in the Makefile we have:
# M= @: do not echo calling make in another directory (quiet mode)
# M= @ echo calling make in another directory (debug / verbose mode)
#
M=
#M= @
But if that variable was set to @:
then many commands would fail as they're prefixed with ${M}
. I believe the purpose though is to make it so that only the invocation is not seen but still invoke it. Certainly that's the case in many instances but that's not what would happen. What would happen is it wouldn't run at all.
But this also goes for the other variables.
An example rule that would fail:
# make parser
#
# Force the rebuild of the JSON parser and then form the reference copies of
# JSON parser C code (if recent enough version of flex and bison are found).
#
parser: jparse/Makefile
${M} ${MAKE} ${MAKE_CD_Q} -C jparse $@
So although it works to silent things more it does it at the cost of disabling the commands entirely though a better example is the one I fixed with build_man
.
Now as far as what to do to fix it.
My thinking is that some of this might want to be removed. The ${M}
for example. Right now the default is empty so it doesn't break any rules but if it's ever changed to @:
or :
it will break the command. At the default value it serves no purpose but it has a false idea that one can simply silent the output more by setting it to @:
.
But on the other hand one can make it more silent by changing it to @
so maybe it's good for that but anything we want to run should not have a variable that starts with either @:
or :
or can be changed to that.
Feel free to assign this to me too but I am not sure what you want done with it. But we can discuss that.
I am going for the night now. Tomorrow I have a zoom meeting but otherwise I should be able to do other things here.
Good night!
Thanks for assigning this to me.
What shall we do?
I think that one way to go about it would be to think about what we want to silent normally and what we don't. Then we can figure out how to make the default work. What is certain is that any variable or literal reference to :
outside of a string will prevent the rule from running (well and outside a command that might use it like grep or cut - you know what I mean here). It might be only if it starts out that way - not sure but I guess so.
What is certain is that if we want lines like:
${V} echo
${V} echo "${OUR_NAME}: make $@ starting"
${V} echo
then we can't have the ${V}
in there if it has the :
or @:
value.
Perhaps we should remove the comments that suggest that this prevents the echoing? Do we even want the V
variable? The Q
seems like it might be good at times.
What about M
? I think it's risky especially if we suggest that having @:
will simply disable the echoing of the command. It would be odd too if the user were to be unable to compile the code normally but then when they try issuing a bug report suddenly it works - because bug_report.sh disables those variables and enables full verbosity.
Maybe this is a good starting point to address this issue.
I have a zoom meeting in about 2h40 minutes and I probably won't do anything else here today but we'll see I guess.
Echo command in actions should be the only actions that start with ${V}
or ${S}
.
Echo command in actions should be the only actions that start with
${V}
or${S}
.
Well that's one thing yes. That was part of the problem I fixed yesterday - some commands had one of those (or was it Q?) and it was preventing commands from being invoked.
But I think there's more to it than this since the M variable also can be a problem - depending on the value.
Echo command in actions should be the only actions that start with
${V}
or${S}
.Well that's one thing yes. That was part of the problem I fixed yesterday - some commands had one of those (or was it Q?) and it was preventing commands from being invoked.
But I think there's more to it than this since the M variable also can be a problem - depending on the value.
Fixing that now ...
Echo command in actions should be the only actions that start with
${V}
or${S}
.Well that's one thing yes. That was part of the problem I fixed yesterday - some commands had one of those (or was it Q?) and it was preventing commands from being invoked.
But I think there's more to it than this since the M variable also can be a problem - depending on the value.Fixing that now ...
Sounds good.
FYI: For most cases, ${MAKE}
lines should use the ${M}
make control.
There are a few exceptions where the use of make
needs to be silent. The build_man
and legacy_clean
and legacy_clobber
Makefile rules need to be silent (for various reasons), so the actions for those Makefile rules need to use ${Q}
instead.
UPDATE 0
As does the rules that are part of the "rules that invoke Makefile rules in other directories" section of Makefiles.
OK. There are a number of cases where make
needs to be silent .. hang on ...
UPDATE 1
Renamed ${M}
to ${E}
. The above so-called make action exceptions use ${E}
now.
FYI: For most cases,
${MAKE}
lines should use the${M}
make control.There are a few exceptions where the use of
make
needs to be silent. Thebuild_man
andlegacy_clean
andlegacy_clobber
Makefile rules need to be silent (for various reasons), so the actions for those Makefile rules need to use${Q}
instead.
As long as those variables aren't :
or @:
anyway. If they are and they're not echo
statements then we will run into a problem.
Changed ${INSTALL_Q}
to just ${I}
.
Changed
${INSTALL_Q}
to just${I}
.
I'll look at this all tomorrow. After that I can give comments on it. Hope you have a great night my friend!
With commit badb5b7 we believe this bug has been resolved.
That's good. I'll take a more careful look tomorrow.
BTW:
Added a final `make all` to prep.sh just before the final
`make test` action. Now doing a `make all` after doing a
`make prep` or `make release` will do / print nothing.
Funny you should say that because I almost did that today ... thanks for taking care of that.
And with that I bid you a good night!