Enhancement: make release and make prep to warn about non-empty Makefile.local files

[lcn2]

Is there an existing issue for this?

• I have searched for existing issues and did not find anything like this

Describe the enhancement

This section has been replaced due to GH-issuecomment-2563052630.

Relevant images, screenshots or other files

n/a

Relevant links

GH-issuecomment-2563052630

Anything else?

Post CODE FREEZE and post-IOCCC28.

All comments and work of this issue to be delayed until after the IOCCC judges's post-IOCCC28 vacation.

[xexyl]

Same thing as the other one ... if you want.

Best wishes!

[xexyl]

Okay I started this with jparse but I can't do that today: the last two days were too hard and long. Tomorrow should be better if I can catch up with other things today.

Anyway I think that this task is not as easy as it seems. The problem is that the script uses the top level Makefile .. but that Makefile invokes rules from other Makefiles which means that those Makefiles probably also have to be moved. This will complicate the script some, possibly more than some.

I'll still look at it in the coming days if I can but just thought I'd remind you of this fact with multiple Makefiles.

Best wishes for a productive day!

[xexyl]

BTW: the -p option does not exist to mv. What are you referring to?

(It doesn't exist in linux or macOS.)

[lcn2]

BTW: the -p option does not exist to mv. What are you referring to?

(It doesn't exist in linux or macOS.)

It was initially cp(1).

But don't bother with anything that is post-IOCCC28 for this repo as they won't be processed for months.

[xexyl]

BTW: the -p option does not exist to mv. What are you referring to?
(It doesn't exist in linux or macOS.)

It was initially cp(1).

But don't bother with anything that is post-IOCCC28 for this repo as they won't be processed for months.

I actually wondered if you meant it to be cp. I won't be making any commit here, that is true, but I might play with the idea in jparse. But other things have to be worked out there too as noted. Not today and perhaps not even in the coming days.

Appreciate the clarification and suggestion.

[lcn2]

We believe we have addressed all of the current questions that still need answering at this time. If we've missed something or something else needs to be clarified, please ask again.

[xexyl]

I noticed a problem with this when I went to do it with jparse.

The problem is that the script under the test directory has its own Makefile but the script makes use of the other Makefile. Also if someone runs make test (or make release etc. which calls make test) both are used.

I suppose the script could go ahead and move both to something like...

mv ../Makefile ..
mv Makefile ..

or so.

But of course this is done in the Makefile so it might not be necessary. Perhaps both Makefiles have this? But I'm still not sure if this will be a conflict in the script.

What do you think?

Anyway since this is now a top priority: does that make it pre IOCCC28?

[lcn2]

I noticed a problem with this when I went to do it with jparse.

The problem is that the script under the test directory has its own Makefile but the script makes use of the other Makefile. Also if someone runs make test (or make release etc. which calls make test) both are used.

I suppose the script could go ahead and move both to something like...

mv ../Makefile ..
mv Makefile ..

or so.

But of course this is done in the Makefile so it might not be necessary. Perhaps both Makefiles have this? But I'm still not sure if this will be a conflict in the script.

What do you think?

The Makefile's are part of the repo, so moving a Makefile and then interrupting the process could leave the repo modified.

We suggest that you try to do it in a script and use a "trap" statement to put it back in case the script is interrupted. It will not project against a "kill -9" but it against the normal interruption of the script if you "trap ... 0 1 2 3 15".

Anyway since this is now a top priority: does that make it pre IOCCC28?

UPDATE 0

It needs to happen before IOCCC28 goes form pending to open .. post 2024 / early 2025.

[xexyl]

Hmm using trap is a good idea ...

I'm not sure the best way to go about this though due to the many subdirectories we descend into and the fact that running make test can work in some directories that aren't the top level directory. In other words there are numerous Makefiles that have to be moved temporarily.

How do you propose to do take care of this? I have to go afk shortly but when back I'll look at the next issue, maybe the UUID one.

[lcn2]

Hmm using trap is a good idea ...

I'm not sure the best way to go about this though due to the many subdirectories we descend into and the fact that running make test can work in some directories that aren't the top level directory. In other words there are numerous Makefiles that have to be moved temporarily.

How do you propose to do take care of this? I have to go afk shortly but when back I'll look at the next issue, maybe the UUID one.

It is beginning to look a lot like a really bad idea for a test to have to move lots of Makefiles out of the way. It might've been unfortunate had it was just one that had to move. But for bunch of then, and just in case something is run from a sub-directory? No.

If the issue is dealing someone running the test from the sub directory, then perhaps the test should detect if it's being run from the sub directory and fail if it doesn't detect, say the .git directory.

Perhaps this issue should be dropped instead?

For now we are taking off the top priority tag. This is you may not be a priority and it's distracting from getting the Great Fork Merge done.

[xexyl]

Hmm using trap is a good idea ...
I'm not sure the best way to go about this though due to the many subdirectories we descend into and the fact that running make test can work in some directories that aren't the top level directory. In other words there are numerous Makefiles that have to be moved temporarily.
How do you propose to do take care of this? I have to go afk shortly but when back I'll look at the next issue, maybe the UUID one.

It is beginning to look a lot like a really bad idea for a test to have to move lots of Makefiles out of the way. It might've been unfortunate had it was just one that had to move. But for bunch of then, and just in case something is run from a sub-directory? No.

If the issue is dealing someone running the test from the sub directory, then perhaps the test should detect if it's being run from the sub directory and fail if it doesn't detect, say the .git directory.

Perhaps this issue should be dropped instead?

For now we are taking off the top priority tag. This is you may not be a priority and it's distracting from getting the Great Fork Merge done.

I agree it might be a bad idea. Maybe instead the test rule can warn about an existing Makefile.local existing ? Just a thought that popped into my head. I don't know if it's the right idea or not.

[lcn2]

I agree it might be a bad idea. Maybe instead the test rule can warn about an existing Makefile.local existing ? Just a thought that popped into my head. I don't know if it's the right idea or not.

Issuing a Warning to the effect that "the existence of Makefile.local might impact .. " may be the best way to go.

Does it have to check just one directory or all of the other directories?

UPDATE 0a

Corrected the text above.

[xexyl]

I agree it might be a bad idea. Maybe instead the test rule can warn about an existing Makefile.local existing ? Just a thought that popped into my head. I don't know if it's the right idea or not.

Issuing a warning that the existence of Makefile.local might impact the ability of the test could be done.

Should that be a warning or merely a notice?

It doesn't matter .. that's kind of what I was thinking of. A notice, a warning, whatever you wish to call it. But as you say below ...

Does it have to check for just one directory or all of the other directories?

I wondered about this too. Problem is there are some fairly deeply nested Makefiles and how far to go? The bug_report.sh does check for these files at least so maybe that's enough?

Issuing a warning to the effect that "the existence of Makefile.local might impact" might be the best way to go. After all the existence of such a file is itself not a problem. It's what's in the file that could be a problem.

In other words you think it might be a good idea? If so the question of which Makefiles, where should it be done and what rules should trigger it? Depending on the answer it could become unmanageable?

Just some random thoughts but if you think it's a good idea we can look at it. But maybe as you say post iOCCC28? After all most people are not going to have overriding Makefiles.

[xexyl]

BTW: the reason I initially put this in was I was making additional checks for the development of jparse and also a lot of extra warnings when making sure the code is stable.

[lcn2]

We edited our comment while you were replying. So you may want to reread our comment above.

[xexyl]

We edited our comment while you were replying. So you may want to reread our comment above.

I replied to the edits in the same message as I saw that. Check the last part. Thanks for the warning though!

[lcn2]

So, if we go with the warning route when doing make release and make prep, you could add a simple shell script to check for the existence of any Makefile.local in any relevant directory where a Makefile exists, that by default exited without any output (unless
Verbose flags were given):

• 0 ==> no non-empty Makefile.local files were found
• 1 ==> some non-empty Makefile.local files were found

We stay non-empty, because an empty Makefile.local file is not going to impact make release nor make prep.

Then make release and make prep could have this code as part of their action:

    @if ! ${CHECK_LOCAL}; then \
        echo "make $@: Warning: Found non-empty Makefile.local file(s)" 1>&2; \
        echo "make $@: ${CHECK_LOCAL} -v 1 output starts below" 1>&2; \
        ${CHECK_LOCAL} -v 1 1>&2; \
        echo "make $@: ${CHECK_LOCAL} -v1 output ends above" 1>&2; \
    fi

We typed the above in by hand, but you see the point hopefully. Without args, ${CHECK_LOCAL} would return false if it found such a non-empty file. With –v 1, ${CHECK_LOCAL} would print information about which files it found that were not empty.

[xexyl]

So, if we go with the warning route when doing make release and make prep, you could add a simple shell script to check for the existence of any Makefile.local in any relevant directory where a Makefile exists, that by default exited without any output (unless Verbose flags were given):

• 0 ==> no non-empty Makefile.local files were found
• 1 ==> some non-empty Makefile.local files were found

We stay non-empty, because an empty Makefile.local file is not going to impact make release nor make prep.

Then make release and make prep could have this code as part of their action:

    @if ! ${CHECK_LOCAL}; then \
        echo "make $@: Warning: Found non-empty Makefile.local file(s)" 1>&2; \
        echo "make $@: ${CHECK_LOCAL} -v 1 output starts below" 1>&2; \
        ${CHECK_LOCAL} -v 1 1>&2; \
        echo "make $@: ${CHECK_LOCAL} -v1 output ends above" 1>&2; \
    fi

We typed the above in by hand, but you see the point hopefully. Without args, ${CHECK_LOCAL} would return false if it found such a non-empty file. With –v 1, ${CHECK_LOCAL} would print information about which files it found that were not empty.

I see what you mean yes. I believe this comes after IOCCC28 though? That would be helpful.

[lcn2]

So, if we go with the warning route when doing make release and make prep, you could add a simple shell script to check for the existence of any Makefile.local in any relevant directory where a Makefile exists, that by default exited without any output (unless Verbose flags were given):

• 0 ==> no non-empty Makefile.local files were found

• 1 ==> some non-empty Makefile.local files were found

We stay non-empty, because an empty Makefile.local file is not going to impact make release nor make prep.

Then make release and make prep could have this code as part of their action:

@if ! ${CHECK_LOCAL}; then \

    echo "make $@: Warning: Found non-empty Makefile.local file(s)" 1>&2; \

    echo "make $@: ${CHECK_LOCAL} -v 1 output starts below" 1>&2; \

    ${CHECK_LOCAL} -v 1 1>&2; \

    echo "make $@: ${CHECK_LOCAL} -v1 output ends above" 1>&2; \

fi

We typed the above in by hand, but you see the point hopefully. Without args, ${CHECK_LOCAL} would return false if it found such a non-empty file. With –v 1, ${CHECK_LOCAL} would print information about which files it found that were not empty.

I see what you mean yes. I believe this comes after IOCCC28 though? That would be helpful.

Correct: after IOCCC28

[lcn2]

We believe we have addressed all of the current questions that still need answering at this time. If we've missed something or something else needs to be clarified, please ask again.

[xexyl]

Correct: after IOCCC28

Thanks! I do think a script might be the best way to go about this but I think it might also require some discussion because there are potentially a number of ways to do it.

But of course as it's post IOCCC28 it's not a big deal right now.

If I do it in the jparse repo (which I will do at some point) it could be adapted or maybe an identical or very close to identical script here.

But as there are a quite a few things that are more important (although at least four should be completed today) this too can wait: kind of like the new tools we need in the jparse repo that we can work on after IOCCC28.

Anyway thanks for clarifying! (It works better for me too.)

[lcn2]

This issue has been resolved with commit 830c805

We needed the not_a_comment.sh script for another thing and realized it could work well
to resolve this issue.

As noted in CHANGES.md:

Both `make prep` and `make release` will issue a notice (via
`soup/not_a_comment.sh`) if a non-empty `Makefile.local` is found
containing more than just comments.  This will allow for one to
keep a `Makefile.local` with only comments in it, without raising
a notice as such a file will not impact the make procedure.
Then if one needs to temporary add comments (perhaps by un-commenting
lines in a `Makefile.local` file), one can.  However if one then
forgets, then the notice will alert you to the potential problem.

[xexyl]

Feel free to apply this to jparse too.

Glad it's resolved. Hopefully tomorrow I can look at that new function.

Sleep time for me. Good night.

[lcn2]

Feel free to apply this to jparse too.

Glad it's resolved. Hopefully tomorrow I can look at that new function.

Sleep time for me. Good night.

The tool used is in the soup directory and the actions are triggered from the top Makefile, so as it is now, it is outside of the scope of the jparse repo.

Perhaps the tool should go into its own repo .. but that would require people to install that tool in order to use it, which kind of defeats the purpose.

[xexyl]

Feel free to apply this to jparse too.
Glad it's resolved. Hopefully tomorrow I can look at that new function.
Sleep time for me. Good night.

The tool used is in the soup directory and the actions are triggered from the top Makefile, so as it is now, it is outside of the scope of the jparse repo.

Perhaps the tool should go into its own repo .. but that would require people to install that tool in order to use it, which kind of defeats the purpose.

But jparse/test_jparse/prep.sh exists .. I'm applying it to the jparse repo now .. testing. If all is good I'll first copy to mkiocccentry/jparse/test_jparse/prep.sh and if all good (make prep in mkiocccentry - which should be fine as it does not run jparse/test_jparse/prep.sh as it it'll only run make test in jparse/) I'll commit to jparse and then sync here.

Thanks!