ioccc-src/mkiocccentry

Bug report: some Makefile variables like ${V} are problematic

Closed this issue ยท 16 comments

xexyl commented

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.

xexyl commented

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.

xexyl commented

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.

xexyl commented

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.

xexyl commented

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.

xexyl commented

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!

xexyl commented

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.

lcn2 commented

Echo command in actions should be the only actions that start with ${V} or ${S}.

xexyl commented

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.

lcn2 commented

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

xexyl commented

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.

lcn2 commented

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.

xexyl commented

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.

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.

lcn2 commented

Changed ${INSTALL_Q} to just ${I}.

xexyl commented

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!

lcn2 commented

With commit badb5b7 we believe this bug has been resolved.

xexyl commented

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!