xexyl commented 1 week ago

Is there an existing issue for this?

[X] I have searched for existing issues and did not find anything like this

Describe the bug

There are a number of things that have to be checked prior to doing a code freeze in this repo prior to Great Fork Merge. These things are best discussed below this OP where comments can be duplicated and then discussed so I am not going to fill them out here.

What you expect

While IOCCC28 is expect to start in late 2024 and the Great Fork Merge needs to happen well before IOCCC28, which means that this has to be done soon.

Environment

- OS: n/a
- Device: n/a
- Compiler: n/a

Yes yes .. I, Cody, made the template and I did not provide what was requested but this is a case of preparing for a code freeze and not necessarily any bug. See the 'Anything else?' section below.

bug_report.sh output

n/a. See below for more details.

Anything else?

It is questionable if this is best in the bug form but since some things might have to be fixed it works. Additionally, there are a variety of things that have to be done and there is not a single category that matches. Not but what I did not want to use the blank form, partly and especially for humour:

The Doors of Obscurity, Lords (Judges) of the IOCCC. Speak, bugs, and squash. I, Cody Boone Ferguson, made them. Landon Curt Noll of the IOCCC didn't draw these signs because there are none.

TODO list

[ ] Fix the bug issue template that has platform etc. The problem is that due to the markdown if someone adds extra lines it is all messed up. Even if they do not add extra lines if the lines are long, because it's markdown, and in particular code blocks, it makes the lines extra long instead of wrapping. A test repo that I used to figure out the issue templates (including the bug template) can be used to figure this out.
[ ] Fix make prep to check for certain tools and if they are not installed or they fail to work on a simple file, then ignore those tests. There are modifications in prep.sh to start this but it seems like another script that should check if a tool exists will be useful which I will work on before I finish prep.sh.

xexyl commented 1 week ago

I hope and know you will enjoy the 'Anything else?' section! Please assign this to me. If you wish to duplicate the relevant comments here please do. I have to do some other things. If I have time I will look at the jparse related issues but if not I'll do that tomorrow.

xexyl commented 1 week ago

I have added the prep.sh script to the jparse repo with some minor adjustments (fixes for the repo). The top level Makefile was missing a MV variable which caused problems. Now I think on it it probably should be checked in the test directory.

I ran into a problem with make test here though. Doing these tasks (in the other repo) I have uncovered some other problems as well and have fixed some things that were missing. There needs to be (in the other repo) a call to the jnum check tool. In this repo it is called in ioccc_test.sh but the obvious choice over there is the jparse_test.sh script. It might also be that it should be removed from ioccc_test.sh and it can be called as part of jparse_test.sh.

QUESTION

But the problem with the tests here has to do with paths being different. Now I got make test in the jparse and jparse/test_jparse directories to work so one solution would be to, once the jnum test has been added, remove the direct calls in ioccc_test.sh and instead have the jparse_test.sh script do the things. Is that okay? If so I hope to do so tomorrow. I unfortunately must get going for the day.

I will need to add, I think, the RUNNING: .. text somewhere but that depends on the script and make rule.

I am trying really hard to get these things done before Saturday (these things include the tasks in this repo) but depending on how the above goes I might need a bit more time. Still I would think that it should not take too much time once we discuss the needed things.

With that said I am leaving for the day. I'm glad that things seem to be working in the jparse repo but still have to get the jnum_ checks over there in and then sort out the ioccc_test.sh here.

xexyl commented 1 week ago

I also noticed that the template should be modified ... look at the platform section. See the long text. It should be, changed to a regular text form I think. I can look at that too.

xexyl commented 1 week ago

Anyway ... back tomorrow. I have a question and progress report up there.

lcn2 commented 1 week ago

Updated name and TODO discuss the Great Fork Merge.

xexyl commented 1 week ago

Updated name and TODO discuss the Great Fork Merge.

Fixed formatting issues so now it looks right.

Seems it might be the form of environment. There was a missing end code block marker.

Also added a missing full stop.

Not just yet but in a while I will look at seeing if I can resolve the jparse to mkiocccentry issue.

Today I likely will be making courgette (or if you prefer zucchini) soup (with courgette from the garden) so that will take some time away from working on these things but I hope to at least resolve that migration issue.

Soon will take a shower and I hope after that I can do a bit but there will be some breaks in between for a number of different things.

Hope you are sleeping well!

xexyl commented 1 week ago

I have fixed make test so that the fixes in the jparse repo now work here. It required removing some tests but those tests are already run from make test in jparse/test_jparse so it was redundant anyway. It might have been possible to use -Z option but this I deem unnecessary except for the case where we run it on the test_ioccc/test_JSON files as the jparse repo knows nothing about those.

Running make test and make release here work fine! I have another minor improvement for the Makefiles here but I'm not sure if I will get to that or not. In any case I'll write the changes and then commit and open a pull request. After this I believe I can start working on documentation here. Unless...

QUESTION

Do you think that the -f option to jparse_test.sh which checks the invalid strings for failure should be run in all cases instead of it being an option? After all the other file, the valid strings one, is always run. If you think that's a good idea I'll do that and then I think we're probably good with jparse here. Unless anything important comes up we can not worry about it until after the next IOCCC.

After the commit I must go afk very possibly for a while. I have other things I have to do when back here too so it's likely I won't get to documentation or this issue until tomorrow. But given that we have to discuss things more first that's not too big of a deal I guess. With that in mind let us discuss what has to be done. Please advise what you think should be done prior to a code freeze!

xexyl commented 1 week ago

Leaving after this but perhaps you would like to copy your comment 2326947197 here? It would be helpful. I think I have a general idea of it but since it's your comment it might be better if you post it?

Sorry for the inconvenience there of course! If you prefer to do it another way that's also fine.

lcn2 commented 1 week ago

QUESTION

But the problem with the tests here has to do with paths being different. Now I got make test in the jparse and jparse/test_jparse directories to work so one solution would be to, once the jnum test has been added, remove the direct calls in ioccc_test.sh and instead have the jparse_test.sh script do the things. Is that okay? If so I hope to do so tomorrow. I unfortunately must get going for the day.

It seems OK, yes.

lcn2 commented 1 week ago

COPIED from GH-issuecomment-2326947197

I guess you mean the bug report script? I can do that soonish.

Yes, but also if someone does a make shellcheck, or a make prep or a make release where the above mentioned broke shellcheck problem exists.

UPDATE 0

... the same way that make depend doesn't "ruin the make, nor the test, nor the prep, nor the build" if someone doesn't have independ installed: in this case if shellcheck fails on the trivial bash script.

Perhaps, as you suggest, the test script could try run the trivial script early on and bail of the trivial bash script failed.

More importantly, the make shellcheck make action must first try to run shellcheck trivial.sh and exit in a "friendly and informative way" AND not do any more shellcheck actions. AND exit 0 if shellcheck trivial.sh were to fail.

UPDATE 1

The test suite could also try the shellcheck trivial.sh case, and if that fails, simply indicate that that the shellcheck test phase is skipped.

lcn2 commented 1 week ago

In regards to GH-issuecomment-2332960965:

When a non-standard non-Unix tool that is used by make prep, make release, or make test fails to process well known and good input because the tool "crashes and burns", then the test should be "Excused".

Please understand we are NOT talking about a case where some Unix command such as cc(1) or grep(1) or sed(1) "crashes and burns". Additionally, we NOT talking about a case where some tool that is part of this repo "crashes and burns".

We are talking about a non-standard non-Unix tools that are NOT part of this repo, such as:

shellcheck(1)
picky(1)
seqcexit(1)
independ(1)

The test code do check if such a tools exist and are executable. However, that is not good enough to warrant their use in tests. Tests should then go one step further and see of the tool to is able to function with known good input, as a sanity check on the tool as well. And here we are NOT talking about Unix utilities, nor are we talking about tools that are built in this repo.

We suggest test code should first see if shellcheck(1) can process, say, soup/trivial.sh (a proposed new file):

#!/usr/bin/env bash
exit 0

If shellcheck soup/trivial.sh runs OK, then go ahead and use shellcheck(1). If, however, it "crashes and burns", then the test that requires shellcheck(1) needs to be "EXCUSED" and not flag each shellcheck(1) use as an error.

We suggest test code should first see if picky(1) can process, say, soup/trivial.c (a proposed new file):

#include <stdio.h>
#include <stdlib.h>

int
main(void)
{
    printf("hello, world\n");
    exit(0);
}

If picky soup/trivial.c runs OK, then go ahead and use picky(1). If, however, it "crashes and burns", then the test that requires picky(1) needs to be "EXCUSED" and not flag each picky(1) use as an error.

Ifseqcexit soup/trivial.c runs OK, then go ahead and use seqcexit(1). If however, it "crashes and burns", then the test that requires seqcexit(1) needs to be "EXCUSED" and not flag each seqcexit(1) use as an error.

If cc -MM soup/trivial.c | independ runs OK, then go ahead and use independ(1). If however. it "crashes and burns", then the test that requires independ(1) needs to be "EXCUSED" and not flag each independ(1) use as an error.

When a test is "EXCUSED", it should NOT look like a failure: it should NOT be a cause for the user to file a bug report.

lcn2 commented 1 week ago

Do you think that the -f option to jparse_test.sh which checks the invalid strings for failure should be run in all cases instead of it being an option? After all the other file, the valid strings one, is always run. If you think that's a good idea I'll do that and then I think we're probably good with jparse here. Unless anything important comes up we can not worry about it until after the next IOCCC.

Sure. 👍

xexyl commented 1 week ago

Do you think that the -f option to jparse_test.sh which checks the invalid strings for failure should be run in all cases instead of it being an option? After all the other file, the valid strings one, is always run. If you think that's a good idea I'll do that and then I think we're probably good with jparse here. Unless anything important comes up we can not worry about it until after the next IOCCC.

Sure. 👍

I agree with this .. will do this I hope sometime this morning. I have to reply to your comment about shellcheck and think about how it might be done for the bug_report.sh script along with any other thing like that that we have to do. After that stuff we can verify that the documentation is good. Fortunately for this repo there isn't that much documentation to go through!

xexyl commented 1 week ago

COPIED from GH-issuecomment-2326947197

I guess you mean the bug report script? I can do that soonish.

Yes, but also if someone does a make shellcheck, or a make prep or a make release where the above mentioned broke shellcheck problem exists.

Anything else other than shellcheck?

UPDATE 0

... the same way that make depend doesn't "ruin the make, nor the test, nor the prep, nor the build" if someone doesn't have independ installed: in this case if shellcheck fails on the trivial bash script.

Perhaps, as you suggest, the test script could try run the trivial script early on and bail of the trivial bash script failed.

More importantly, the make shellcheck make action must first try to run shellcheck trivial.sh and exit in a "friendly and informative way" AND not do any more shellcheck actions. AND exit 0 if shellcheck trivial.sh were to fail.

Okay. This seems reasonable. Only thing is .. which make shellcheck? The top level Makefile? Or maybe all of them? Perhaps it runs shellcheck on stdout of a simple script? Do we need a script shellcheck.sh? I'd be happy to do that. Maybe it's simply a matter of running shellcheck on each arg passed. I could do this for the jparse repo too.

That sounds like a good idea .. will play with it in a while.

UPDATE 1

The test suite could also try the shellcheck trivial.sh case, and if that fails, simply indicate that that the shellcheck test phase is skipped.

That makes sense but then since it runs make shellcheck (I think) there's no need to do that specially?

xexyl commented 1 week ago

COPIED from GH-issuecomment-2326947197

I guess you mean the bug report script? I can do that soonish.

Yes, but also if someone does a make shellcheck, or a make prep or a make release where the above mentioned broke shellcheck problem exists.

Anything else other than shellcheck?

UPDATE 0

... the same way that make depend doesn't "ruin the make, nor the test, nor the prep, nor the build" if someone doesn't have independ installed: in this case if shellcheck fails on the trivial bash script.

Perhaps, as you suggest, the test script could try run the trivial script early on and bail of the trivial bash script failed.

More importantly, the make shellcheck make action must first try to run shellcheck trivial.sh and exit in a "friendly and informative way" AND not do any more shellcheck actions. AND exit 0 if shellcheck trivial.sh were to fail.

Okay. This seems reasonable. Only thing is .. which make shellcheck? The top level Makefile? Or maybe all of them? Perhaps it runs shellcheck on stdout of a simple script? Do we need a script shellcheck.sh? I'd be happy to do that. Maybe it's simply a matter of running shellcheck on each arg passed. I could do this for the jparse repo too.

That sounds like a good idea .. will play with it in a while.

UPDATE 1

The test suite could also try the shellcheck trivial.sh case, and if that fails, simply indicate that that the shellcheck test phase is skipped.

That makes sense but then since it runs make shellcheck (I think) there's no need to do that specially?

xexyl commented 1 week ago

A downside with a script to run on files is something like...

$ shellcheck run_shellcheck.sh 

In run_shellcheck.sh line 113:
if [[ "$?" -ne 0 ]]; then
      ^--^ SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.

For more information:
  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...

versus:

$ ./run_shellcheck.sh run_shellcheck.sh 
run_shellcheck.sh:113:7: note: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?. [SC2181]

I wonder now what happens with make shellcheck in the case of a failure.

QUESTION

Is that a problem?

--

Anyway I'm really tired and can't do more right now so I'll return to this in a while. If nothing else today I'll do the updates in those repos and then sync everything here. But for now I have to go.

xexyl commented 1 week ago

Hmm ... a downside to using the script is the Makefile has to know where the script is and as it won't be installed it won't work like the Makefile just using SHELLCHECK= shellcheck ....

So maybe it should not be a script. Maybe just the rule can be updated instead. Okay well I'll play with this when I'm back which might not be for a while. Not sure. I have other things I have to do today too.

xexyl commented 1 week ago

Looking at your more detailed comment I think I'll start over with this when I'm back (and hopefully more awake). I might first do the jparse test script update and then look at this stuff. Hopefully I can make good progress today. But for now I really do need to go afk.

xexyl commented 1 week ago

Hmm ... okay I'm wondering if having a script like ioccc_test.sh would be a good location for these things, perhaps test_ioccc/make.sh. It could have different modes that could be run. I wonder.

A downside here is that for some rules it needs file paths. On the other hand it would be easier to maintain, maybe, since some of these rules are already quite complicated and not only that there are many that have have very similar or identical rules in Makefiles it uses with make -C.

I'm going to look at the jparse thing first and the other repos (the .gitignore and also the Makefile enhancements) and see what I feel after that. There will be a time where I have to go afk, quite soon in fact, and I do have other things I have to do today but I hope to get these things done at least and perhaps some of this task done too.

lcn2 commented 1 week ago

A downside with a script to run on files is something like...

$ shellcheck run_shellcheck.sh 

In run_shellcheck.sh line 113:
if [[ "$?" -ne 0 ]]; then
      ^--^ SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.

For more information:
  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...

versus:

$ ./run_shellcheck.sh run_shellcheck.sh 
run_shellcheck.sh:113:7: note: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?. [SC2181]

I wonder now what happens with make shellcheck in the case of a failure.

QUESTION

Is that a problem?

ANSWER

Yes, the above code is a problem. We use the following:

.. some command we want to check on ..
status="$?"
if [[ $status -ne 0 ]]; then
   echo "$0: ERROR: some command failed," \
        "error: $status" 1>&2
   ... stuff, perhaps exit, perhaps set a variable to exit non-zero later, etc. ...
fi

Capture the status in a variable called $status, then test by doingif [[ $status -ne 0 ]]; .. (notice no quotes).

If ".. some command we want to check on .." is a pipe of commands, then: do this:

command | command2 arg | fizzbin -curds -whey
status_codes=("${PIPESTATUS[@]}")
if [[ ${status_codes[*]} =~ [1-9] ]]; then
   echo "$0: ERROR: command | command2 arg | fizzbin -curds -whey failed," \
        "error codes: ${status_codes[*]}" 1>&2
  ... stuff, perhaps exit, perhaps set a variable to exit non-zero later, etc. ...
fi

UPDATE 0a

Fixed cut and paste errors.

xexyl commented 1 week ago

A downside with a script to run on files is something like...

$ shellcheck run_shellcheck.sh 

In run_shellcheck.sh line 113:
if [[ "$?" -ne 0 ]]; then
      ^--^ SC2181 (style): Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?.

For more information:
  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...

versus:

$ ./run_shellcheck.sh run_shellcheck.sh 
run_shellcheck.sh:113:7: note: Check exit code directly with e.g. 'if ! mycmd;', not indirectly with $?. [SC2181]

I wonder now what happens with make shellcheck in the case of a failure.

QUESTION

Is that a problem?

ANSWER

Yes, the above code is a problem. We use the following:

.. some command we want to check on ..
status="$?"
if [[ $status -ne 0 ]]; then
   echo "$0: ERROR: some command failed," \
        "error: $status" 1>&2
   ... stuff, perhaps exit, perhaps set a variable to exit non-zero later, etc. ...
fi

Capture the status in a variable called $status, then test by doingif [[ $status -ne 0 ]]; .. (notice no quotes).

If ".. some command we want to check on .." is a pipe of commands, then: do this:

command | command2 arg | fizzbin -curds -whey
status_codes=("${PIPESTATUS[@]}")
if [[ ${status_codes[*]} =~ [1-9] ]]; then
   echo "$0: ERROR: command | command2 arg | fizzbin -curds -whey failed," \
        "error codes: ${status_codes[*]}" 1>&2
  ... stuff, perhaps exit, perhaps set a variable to exit non-zero later, etc. ...
fi

Well I can do that but I thought you wanted it to not be an error if these simple tests fail? Rather they should just be ignored? I'm talking about the trivial files.

lcn2 commented 1 week ago

Well I can do that but I thought you wanted it to not be an error if these simple tests fail? Rather they should just be ignored? I'm talking about the trivial files.

BTW: We fixed the quoted text, see UPDATE 0a above.

As you your implies question:

We are unclear as to your context, so it is hard to answer. You may be under a mis-conception as well.

Please review GH-issuecomment-2333047790.

In the context of GH-issuecomment-2333047790, you might do something like:

# Determine if shellcheck works
#
export EXCUSE_TEST=""
shellcheck soup/trivial.sh >/dev/null 2>&1
status="$?"
if [[ $status -ne 0 ]]; then
    # shellcheck crashed and burned, we cannot use it to test
    EXCUSE_TEST="true"
fi

Then:

if [[ -n $EXCUSE_TEST ]]; then
    .. print EXCUSED (instead of PASS or FAIL) .. 
else
    .. run tests using shellcheck and report PASS or FAIL ..
fi

Or something like that.

xexyl commented 1 week ago

Well I can do that but I thought you wanted it to not be an error if these simple tests fail? Rather they should just be ignored? I'm talking about the trivial files.

BTW: We fixed the quoted text, see UPDATE 0a above.

As you your implies question:

We are unclear as to your context, so it is hard to answer. You may be under a mis-conception as well.

I guess it's more like a misunderstanding. See below.

Please review GH-issuecomment-2333047790.

Yes. I have. That's where the confusion comes from.

In the context of GH-issuecomment-2333047790, you might do something like:

# Determine if shellcheck works
#
export EXCUSE_TEST=""
shellcheck soup/trivial.sh >/dev/null 2>&1
status="$?"
if [[ $status -ne 0 ]]; then
    # shellcheck crashed and burned, we cannot use it to test
    EXCUSE_TEST="true"
fi

Then:

if [[ -n $EXCUSE_TEST ]]; then
    .. print EXCUSED (instead of PASS or FAIL) .. 
else
    .. run tests using shellcheck and report PASS or FAIL ..
fi

Or something like that.

Right. But the other comment, the one you made a short bit ago, suggested (as I interpreted it anyway) that it should be an error if the trivial tests fail. But perhaps you were not thinking of the trivial files? Nonetheless the print statement is of use.

Summary - please correct if necessary

My understanding is that if a test on a trivial file fails it can be excused. If it fails on other files it obviously is an error that should be reported.

I think also that maybe the excused message should only be printed depending on debug level (of course if in Makefile that's another matter entirely).

Question

If a tool is not found should it print an excused message? I guess it might be more useful to print out what it always has done but maybe you've changed your idea?

UPDATE 0

On second thoughts (how does that work?) maybe it is better to print the excused message but perhaps the text can be changed a bit.

xexyl commented 1 week ago

Okay more changes to jparse were necessary. See https://github.com/xexyl/jparse/commit/c39a53d883efbd403b546a1a72f0dbe43f50e43b.

Do you want this put in this repo?

Have to take care of other things now. Will look at documentation or something here if I have time. Otherwise tomorrow. I'm afraid that the time this has taken means we'll need a few more days to get the code freeze in place but depending on discussion speed I'm sure we can manage by next week!

lcn2 commented 1 week ago

Do you want this put in this repo?

Yes. It is better to sync something, than to be inconsistent, thanks!

lcn2 commented 1 week ago

We added a TODO to complete this issue in issue #2239 in the "other repo".

lcn2 commented 1 week ago

Summary - please correct if necessary

My understanding is that if a test on a trivial file fails it can be excused. If it fails on other files it obviously is an error that should be reported.

Correct. And by "it can be excused" we mean that ALL the other tests using the tool are "EXCUSED".

I think also that maybe the excused message should only be printed depending on debug level (of course if in Makefile that's another matter entirely).

Sure.

Question

If a tool is not found should it print an excused message? I guess it might be more useful to print out what it always has done but maybe you've changed your idea?

If the tool is not found, then there is no point in running any of the tests.

UPDATE 0

On second thoughts (how does that work?) maybe it is better to print the excused message but perhaps the text can be changed a bit.

We have 4 states for a "make_action"

OK
ERROR
EXCUSED
MISSING

Let's take shellcheck(1) for example. Here is what to do for all 4 of those states:

OK

If shellcheck(1):

exists
executable
trivial test passes
all tests in the test suite passes

then print:

make_action 25 shellcheck OK

ERROR

If shellcheck(1):

exists
executable
trivial test passes
one or more of the tests in the test suite fails:

then print:

make_action 25 shellcheck ERROR exit code ((number))

and at the end print:

One or more tests failed:

    make_action 25: /Library/Developer/CommandLineTools/usr/bin/make -f ./Makefile shellcheck: non-zero exit code: 2

One or more tests failed.

See test_ioccc/test_ioccc.log for more details.

make prep: ERROR: ./test_ioccc/prep.sh exit code: 25

make prep: see build.log for details

EXCUSED

If shellcheck(1):

exists
executable
trivial test fails

NOTE: We skip all tests in the test suite that use shellcheck(1).

then print:

make_action 25 shellcheck EXCUSED

and at the end print:

 One or more tests were excused:

    make_action 25: The shellcheck tool is unreliable on your system

We cannot use the shellcheck tool  that is installed on your system.
Please consider updating and re-installing your shellcheck tool from:

        https://github.com/koalaman/shellcheck.net

or if needed, filing a bug report with those who publish shellcheck.

Please do NOT file a bug report with us as the IOCCC does not maintain shellcheck.

MISSING

If shellcheck(1):

does nor exist OR
is not executable

then print:

make_action 25 shellcheck MISSING

and at the end print:

One or more tests could not be run because an executable tool was not found.

    make_action 25: The shellcheck tool is missing or is not executable

Please consider installing your shellcheck tool from:

        https://github.com/koalaman/shellcheck.net

and be sure that the shellcheck tool is in your $PATH.

p.s.

A similar thing can be done for seqcexit(1) for "make_action 26 seqcexit". Use the URL: https://github.com/lcn2/seqcexit for the site.

A similar thing can be done for picky(1) for "make_action 27 picky". Use the URL: https://github.com/lcn2/picky for the site.

A similar thing can be done for independ(1) for "make_action 12 depend". Use the URL: https://github.com/lcn2/independ for the site.

p.p.s.

Even though the last 3 repo URLs are under the lcn2 account, they are NOT IOCCC related, so printing "Please do NOT file a bug report with us as the IOCCC does not maintain ..." when the test is EXCUSED is just fine.

lcn2 commented 1 week ago

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 commented 1 week ago

Do you want this put in this repo?

Yes. It is better to sync something, than to be inconsistent, thanks!

Done. Included the fix to error messages too!

xexyl commented 1 week ago

We added a TODO to complete this issue in issue #2239 in the "other repo".

I looked at it ... thanks!

xexyl commented 1 week ago

As far as comment 2335054311 there is a lot to consider and I believe I might still have some uncertainties. I'll try asking now but then I must go do other things. Hopefully I have time later on to get some of it done but it might also depend on if anything can be done without further clarification.

xexyl commented 1 week ago

As far as comment 2335054311 there is a lot to consider and I believe I might still have some uncertainties. I'll try asking now but then I must go do other things. Hopefully I have time later on to get some of it done but it might also depend on if anything can be done without further clarification.

Actually ... looking at the comment again I think that you refer to the prep.sh script. Okay that makes much more sense and is easier to do too.

In that case I'm not sure I need to ask any questions. I will have to look at this later - hopefully today but if not then I should be able to tomorrow as I should have less things to address.

xexyl commented 1 week ago

As far as comment 2335054311 there is a lot to consider and I believe I might still have some uncertainties. I'll try asking now but then I must go do other things. Hopefully I have time later on to get some of it done but it might also depend on if anything can be done without further clarification.

Actually ... looking at the comment again I think that you refer to the prep.sh script. Okay that makes much more sense and is easier to do too.

In that case I'm not sure I need to ask any questions. I will have to look at this later - hopefully today but if not then I should be able to tomorrow as I should have less things to address.

On second thoughts .. maybe it's not this simple. The make_action actually relies on the Makefile itself. So it requires maybe different exit codes from the Makefile. Which means complicating the Makefiles a fair amount. Additionally if a check is added to the top level Makefile shouldn't it be added to the subdirectories if their Makefile also has that rule?

It seems like that might be what is necessary. And as for checking trivia scripts/code I'm not sure: it might be better to just check in prep.sh itself.

QUESTION

It might be possible to not complicate the Makefile even more. Perhaps the script could check for these things and only run the action if it can be run? In the case it can't it can show excused or whatever (I think IGNORED might be better though, personally). This however will mean extra functions in the script will have to be run. For instance any tool that we have to make extra checks will need a new function that will either run the make_action function or else report in the log (and/or output) the status. In other words it's a special version of the make_action function for the tools itself.

Since this is for prep.sh I think that this would be the cleanest way to go about it esp as it would then mean we do not need to worry about complicating the already complicated Makefile rules.

What do you think about doing that?

xexyl commented 1 week ago

Okay I started this task with prep.sh. I added the functions which are at this point a copy of make_action. The script calls the new functions. Now it's a matter of updating each one to first make the checks of the tools existing and in the case where a trivial script/file has to be checked I can add that in too.

I suspect this should not be that difficult to do. I don't know if it's one sitting - it depends on the time and what else is going on but it could be one sitting.

But now I must do other things. Back either later today or tomorrow.

xexyl commented 1 week ago

I will resume this tomorrow. Have to go now. Good day!

lcn2 commented 1 week ago

QUESTION It might be possible to not complicate the Makefile even more. Perhaps the script could check for these things and only run the action if it can be run? In the case it can't it can show excused or whatever (I think IGNORED might be better though, personally). This however will mean extra functions in the script will have to be run. For instance any tool that we have to make extra checks will need a new function that will either run the make_action function or else report in the log (and/or output) the status. In other words it's a special version of the make_action function for the tools itself.

Since this is for prep.sh I think that this would be the cleanest way to go about it esp as it would then mean we do not need to worry about complicating the already complicated Makefile rules.

What do you think about doing that?

ANSWER

We agree. This doesn't have to be implemented in a Makefile per se. The prep.sh is probably the best place to from functionality suggested by GH-.

One possible thing that prep.sh might do is to provide an easy way for a Makefile to ask if a tool should be "EXCUSED". For example, and this is just a wild guess ... assume it has a -A flag to test if one of the 4 tools is "available":

prep.sh -A {shellcheck,picky,independ,seqcexit}

All that does is to exit 0 if the tool:

exists
is executable
passed the trivial data test

otherwise it exists non-0.

Doing the above would make it very easy for a rule such as make picky to test if the requisite tool is ready for use, or to avoid it.

Of course, you could move this functionality into a separate shell scripts, such as soup/is_available.sh and let both prep.sh (and friends) as well as Makefiles use it. As such it could be just:

soup/is_available.sh {shellcheck,picky,independ,seqcexit}

That would just run an appropriate exit code.

xexyl commented 1 week ago

QUESTION It might be possible to not complicate the Makefile even more. Perhaps the script could check for these things and only run the action if it can be run? In the case it can't it can show excused or whatever (I think IGNORED might be better though, personally). This however will mean extra functions in the script will have to be run. For instance any tool that we have to make extra checks will need a new function that will either run the make_action function or else report in the log (and/or output) the status. In other words it's a special version of the make_action function for the tools itself. Since this is for prep.sh I think that this would be the cleanest way to go about it esp as it would then mean we do not need to worry about complicating the already complicated Makefile rules. What do you think about doing that?

ANSWER

We agree. This doesn't have to be implemented in a Makefile per se. The prep.sh is probably the best place to from functionality suggested by GH-.

Right. I will do that and from what I started yesterday. Unfortunately fixing and figuring out the additional problems you discovered took too much time and I need to take care of some other things now. It is hoped tomorrow I can finish this task and then maybe go on to the documentation part in the other issue.

I will warn you though that tomorrow, Monday, I have a zoom call, and Tuesday I have a doctor appointment, so there will be some time where I won't be able to do that. I also likely will be baking in the next few days (once it's cooled down a little bit .. though certainly not cool).

One possible thing that prep.sh might do is to provide an easy way for a Makefile to ask if a tool should be "EXCUSED". For example, and this is just a wild guess ... assume it has a -A flag to test if one of the 4 tools is "available":
prep.sh -A {shellcheck,picky,independ,seqcexit}
All that does is to exit 0 if the tool:

exists

is executable

passed the trivial data test

otherwise it exists non-0.

Doing the above would make it very easy for a rule such as make picky to test if the requisite tool is ready for use, or to avoid it.

Of course, you could move this functionality into a separate shell scripts, such as soup/is_available.sh and let both prep.sh (and friends) as well as Makefiles use it. As such it could be just:
soup/is_available.sh {shellcheck,picky,independ,seqcexit}
That would just run an appropriate exit code.

At first I was not sure of the new script but after reading it a second time (or rather once I got to the top .. I read it bottom to top) it seems like a good idea. The script would have to be added to the jparse repo and it might be good to add it to the other repos too. I can do this as part of my work here. That being said it would still require some new functions as it then has to give different messages (and return early from the function if necessary) based on if the tool is available or not.

The one downside for this method though is that the Makefiles have to determine the path of the script and maybe have to do ../soup. We could have the top level Makefile not go into the other Makefiles if a tool is not found but even if we do that (which seems to not be a bad idea) the fact is someone could do the command from the directory themselves.

As for jparse having this type of script and the other two repos: this means that in those directories their respective Makefiles will have to use their tools, as obviously jparse and the others know nothing about soup/. This should not be a big problem of course but it is something I'd have to do. I likely could copy the script over almost exactly if not exactly and then just have slightly different make targets.

QUESTION

How does that all sound to you?

--

I have to go for now and probably the rest of the day but as long as nothing else comes up here I should be able to start working on this tomorrow.

xexyl commented 1 week ago

As for the doctor appointment: it's the podiatrist. Which brings up an old joke / pun of mine which you might enjoy:

Some things cost an arm and a leg. A podiatrist costs a leg and a foot.

xexyl commented 1 week ago

Added a todo list in the top comment. I must go now. Back tomorrow!

lcn2 commented 1 week ago

The one downside for this method though is that the Makefiles have to determine the path of the script and maybe have to do ../soup. We could have the top level Makefile not go into the other Makefiles if a tool is not found but even if we do that (which seems to not be a bad idea) the fact is someone could do the command from the directory themselves

The top Makefile can simple use soup/is_available.sh whereas something like soup/Makefile, could use ./is_available.sh and test_ioccc/Makefile if it needs to, can use ../soup/is_available.sh. And/or your idea of controlling it at the top level is also reasonable.

xexyl commented 1 week ago

The one downside for this method though is that the Makefiles have to determine the path of the script and maybe have to do ../soup. We could have the top level Makefile not go into the other Makefiles if a tool is not found but even if we do that (which seems to not be a bad idea) the fact is someone could do the command from the directory themselves

The top Makefile can simple use soup/is_available.sh whereas something like soup/Makefile, could use ./is_available.sh and test_ioccc/Makefile if it needs to, can use ../soup/is_available.sh. And/or your idea of controlling it at the top level is also reasonable.

Yes. I'll do that.

As for the other repos the appropriate paths in the Makefile (each one) will take care of it. Should not be a problem.

lcn2 commented 1 week ago

As for jparse having this type of script and the other two repos: this means that in those directories their respective Makefiles will have to use their tools, as obviously jparse and the others know nothing about soup/. This should not be a big problem of course but it is something I'd have to do. I likely could copy the script over almost exactly if not exactly and then just have slightly different make targets.

Fair point.

One hack could be to not bother with that tool in the jparse repo and let the upper level Makefile control it.

Another idea is to let the jparse repo have and use its own is_available.sh script. Such a script might be useful for non-developers who want to use the result of the jparse repo and just to a make all without having to worry about auxiliary tools such as picky, and friends.

QUESTION

How does that all sound to you?

Sound like you have some good ideas to consider and implement!

lcn2 commented 1 week ago

Some things cost an arm and a leg. A podiatrist costs a leg and a foot.

In the rest of the world, they meter 📏 out the cost. 🤓

xexyl commented 1 week ago

Some things cost an arm and a leg. A podiatrist costs a leg and a foot.

In the rest of the world, they meter 📏 out the cost. 🤓

Or meat out the cost ... yes yes I know it's 'mete' but that would ruin the (admittedly sordid) pun.

xexyl commented 1 week ago

As for jparse having this type of script and the other two repos: this means that in those directories their respective Makefiles will have to use their tools, as obviously jparse and the others know nothing about soup/. This should not be a big problem of course but it is something I'd have to do. I likely could copy the script over almost exactly if not exactly and then just have slightly different make targets.

Fair point.

One hack could be to not bother with that tool in the jparse repo and let the upper level Makefile control it.

Another idea is to let the jparse repo have and use its own is_available.sh script. Such a script might be useful for non-developers who want to use the result of the jparse repo and just to a make all without having to worry about auxiliary tools such as picky, and friends.

Yes it would have its own. That was the idea. Only because if I have a similar script (and it's a good idea) to prep.sh (same name in fact) over there then I should add these features too, for the same reasons.

QUESTION

How does that all sound to you?

Sound like you have some good ideas to consider and implement!

Thanks. I will.

lcn2 commented 1 week ago

WARNING: Reversal of a previous position

We need to back out of our statements about "leaf Makefile" not issuing "starting" and "ending" messages: Sorry (tm Canada 🇨🇦).

In actions such as make clobber, make clean, make uninstall, silently removing a file is NOT a good thing, nor it is a nice thing to do.

We plan to undo previously suggested changes to the dbg repo and the dyn_array repo so that such removal actions are declared to the user. And if you are going to print:

rm  -f dbg.o dbg_test.o dbg_example.o dbg_test.c
rm  -f dbg_test.out

You might as well print:

dbg: make clean starting

rm  -f dbg.o dbg_test.o dbg_example.o dbg_test.c
rm  -f dbg_test.out

dbg: make clean ending

And for those who complain about such "verbosity", such echos at the start and end of a rule are controlled by ${S}. Why ${S} and not ${E}? Well because if such messages bother you and you want them to "go away", then simply set S=@# as in:

make clobber S=@#

and you will just get:

rm  -f dbg.a
rm  -f dbg.o dbg_test.o dbg_example.o dbg_test.c
rm  -f dbg_test.out
rm  -f libdbg.a dbg_test dbg_example
rm  -f tags .local.dir.tags
rm  -f Makefile.orig

And if you want to be kept "completely in the dark as to what is happening" you can always do:

make clobber S=@# E=@#

We will begin undoing the incorrectly assumed "not in leaf Makefiles" position with some commits.

Well, one sometimes needs to be reminder WHY there are certain best practices in Makefile. Thanks for your understanding and sorry (tm Canada 🇨🇦) for the confusion.

UPDATE 0

There are exceptions, such as when one has a complex command in an action where one doesn't to the full details of something like a test, so instead explicit echo command are added. For example:

        ${Q} if [[ -d jparse.clone ]]; then \
            echo "ERROR: jparse.clone exists"; \
            exit 1; \
        else \
            echo "If git clone fails because you do not have the ssh key, try:"; \
            echo; \
            echo "      ${GIT} clone https://github.com/xexyl/jparse.git jparse.clone"; \
            echo; \
            ${GIT} clone git@github.com:xexyl/jparse.git jparse.clone; \
        fi

Another exception is where the removal of a file is being done prior to that file being overwritten. For example in the make local_dir_tags actions, where the "${LOCAL_DIR_TAGS}" is going to be overwritten, and a removal is being done out of paranoia:

        ${Q} ${RM} -f ${LOCAL_DIR_TAGS}
        -${E} ${CTAGS} -w -f ${LOCAL_DIR_TAGS} ${ALL_CSRC} ${ALL_HSRC}

The removal of ${LOCAL_DIR_TAGS} is not strictly need to be shown as the ${CTAGS} command overwrites the file.

In general, silencing of a command, especially a removal of something, should be done sparingly and only with good reason.

UPDATE 0

In undoing the prior "not in a leaf Makefile" position, we discovered a minor issue issue with make depened in this repo. Those "starting" and "ending" statements are proving useful to fix the minor issue.

lcn2 commented 1 week ago

We will now "sync" the sub-trees in this repo that are obtained from other repos. As a reminder, this may be done with:

make all.recreate_clone && make all.update_from_clone

xexyl commented 1 week ago

WARNING: Reversal of a previous position

We need to back out of our statements about "leaf Makefile" not issuing "starting" and "ending" messages: Sorry (tm Canada 🇨🇦).

In actions such as make clobber, make clean, make uninstall, silently removing a file is NOT a good thing, nor it is a nice thing to do.

We plan to undo previously suggested changes to the dbg repo and the dyn_array repo so that such removal actions are declared to the user. And if you are going to print:
rm  -f dbg.o dbg_test.o dbg_example.o dbg_test.c

rm  -f dbg_test.out
You might as well print:
dbg: make clean starting

rm  -f dbg.o dbg_test.o dbg_example.o dbg_test.c

rm  -f dbg_test.out

dbg: make clean ending
And for those who complain about such "verbosity", such echos at the start and end of a rule are controlled by ${S}. Why ${S} and not ${E}? Well because if such messages bother you and you want them to "go away", then simply set S=@# as in:
make clobber S=@#
and you will just get:
rm  -f dbg.a

rm  -f dbg.o dbg_test.o dbg_example.o dbg_test.c

rm  -f dbg_test.out

rm  -f libdbg.a dbg_test dbg_example

rm  -f tags .local.dir.tags

rm  -f Makefile.orig
And if you want to be kept "completely in the dark as to what is happening" you can always do:
make clobber S=@# E=@#
We will begin undoing the incorrectly assumed "not in leaf Makefiles" position with some commits.

Well, one sometimes needs to be reminder WHY there are certain best practices in Makefile. Thanks for your understanding and sorry (tm Canada 🇨🇦) for the confusion.

UPDATE 0

There are exceptions, such as when one has a complex command in an action where one doesn't to the full details of something like a test, so instead explicit echo command are added. For example:
        ${Q} if [[ -d jparse.clone ]]; then \

            echo "ERROR: jparse.clone exists"; \

            exit 1; \

        else \

            echo "If git clone fails because you do not have the ssh key, try:"; \

            echo; \

            echo "      ${GIT} clone https://github.com/xexyl/jparse.git jparse.clone"; \

            echo; \

            ${GIT} clone git@github.com:xexyl/jparse.git jparse.clone; \

        fi
Another exception is where the removal of a file is being done prior to that file being overwritten. For example in the make local_dir_tags actions, where the "${LOCAL_DIR_TAGS}" is going to be overwritten, and a removal is being done out of paranoia:
        ${Q} ${RM} -f ${LOCAL_DIR_TAGS}

        -${E} ${CTAGS} -w -f ${LOCAL_DIR_TAGS} ${ALL_CSRC} ${ALL_HSRC}
The removal of ${LOCAL_DIR_TAGS} is not strictly need to be shown as the ${CTAGS} command overwrites the file.

In general, silencing of a command, especially a removal of something, should be done sparingly and only with good reason.

UPDATE 0

In undoing the prior "not in a leaf Makefile" position, we discovered a minor issue issue with make depened in this repo. Those "starting" and "ending" statements are proving useful to fix the minor issue.

No need to be sorry. Actually I can see it both ways.

I am unsure if I did it in jparse. I think I might not have. If that's the case then all is good; otherwise I will do it tomorrow.

xexyl commented 1 week ago

We will now "sync" the sub-trees in this repo that are obtained from other repos. As a reminder, this may be done with:
make all.recreate_clone && make all.update_from_clone

I hadn't noticed the all. version. Thanks!

Although I have been using the update clone and then update from clone rules.

Should I use the recreate ones?

lcn2 commented 1 week ago

We will now "sync" the sub-trees in this repo that are obtained from other repos. As a reminder, this may be done with:
make all.recreate_clone && make all.update_from_clone
I hadn't noticed the all. version. Thanks!

Although I have been using the update clone and then update from clone rules.

Should I use the recreate ones?

We just did that with commit 697902f29fd72cfd66865376ca0ba4216fc37595.

We are now returning to an issue in the other repo related ti bin tools and XPath for JSON.

ioccc-src / mkiocccentry

Enhancement: prepare for code freeze prior to Great Fork Merge #956

Is there an existing issue for this?

Describe the bug

What you expect

Environment

bug_report.sh output

Anything else?

TODO list

QUESTION

QUESTION

QUESTION

COPIED from GH-issuecomment-2326947197

UPDATE 0

UPDATE 1

COPIED from GH-issuecomment-2326947197

UPDATE 0

UPDATE 1

COPIED from GH-issuecomment-2326947197

UPDATE 0

UPDATE 1

QUESTION

QUESTION

ANSWER

UPDATE 0a

QUESTION

ANSWER

Summary - please correct if necessary

Question

UPDATE 0

Summary - please correct if necessary

Question

UPDATE 0

OK

ERROR

EXCUSED

MISSING

p.s.

p.p.s.

QUESTION

ANSWER

ANSWER

QUESTION

QUESTION

QUESTION

WARNING: Reversal of a previous position

UPDATE 0

UPDATE 0

WARNING: Reversal of a previous position

UPDATE 0

UPDATE 0