ioccc-src / mkiocccentry

Form an IOCCC submission as a compressed tarball file
Other
28 stars 6 forks source link

Enhancement: create the chkentry tool #259

Closed xexyl closed 1 year ago

xexyl commented 2 years ago

We can discuss the details here. If you've not responded to the question on . in the other thread before I get a chance to (too tired right now) I will copy and paste my reply here to discuss it here. Otherwise I'll move my reply to your new reply here.

Please add the tags you want and also assign it to me. You can then close the other issues and refer to this one here as you suggested.

I don't know when I can start working on this but the first part will be writing main() and a sanity check function. This most likely will not happen today I'm afraid. Not even sure if I'll get anything done today here. I'm hoping I can get some rest soon but last time I tried earlier this morning I couldn't.

Later on I will wake up more but whether I do more here is yet to be seen. Anyway we can now discuss the new tool here which should also help remove some clutter elsewhere (or prevent further clutter I guess).

lcn2 commented 2 years ago

I am looking at the man page of chkentry and have made some minor fixes but II'm unsure of something and wondering if you can enlighten me on it.

We have:

SYNOPSIS
       chkentry [-h] [-v level] [-J level] [-V] [-q] [-F fnamchk] [-w] entry_dir
       chkentry [-h] [-v level] [-J level] [-V] [-q] [-F fnamchk] [-w] info.JSON author.JSON

But I don't see the -w in the OPTIONS section and it's not in getopt() call in the source file either. Any idea what this was supposed to be for? Thanks.

Our guess is that this was a copy and paste error, and so -w should be removed (unless / until we have a reason for it).

xexyl commented 2 years ago

I am looking at the man page of chkentry and have made some minor fixes but II'm unsure of something and wondering if you can enlighten me on it. We have:

SYNOPSIS
       chkentry [-h] [-v level] [-J level] [-V] [-q] [-F fnamchk] [-w] entry_dir
       chkentry [-h] [-v level] [-J level] [-V] [-q] [-F fnamchk] [-w] info.JSON author.JSON

But I don't see the -w in the OPTIONS section and it's not in getopt() call in the source file either. Any idea what this was supposed to be for? Thanks.

Our guess is that this was a copy and paste error, and so -w should be removed (unless / until we have a reason for it).

This was my guess as well. I will do that.

I think you'll also be happy about what I get in. I do have a minor fix to foo too which I'll throw in as well since I'll be making a commit anyway. Some of what I'm doing probably will have to change somewhat but it's a good start. Well you'll see. Anyway I'll remove that option from the man page. Thanks!

lcn2 commented 2 years ago

Another thing. In chkentry.h we have:

#define MATCH_PRECISION ((long double)(1<<22))

But this same definition is in jnum_chk.h. Should chkentry.h #include "jnum_chk.h" or should the definition be removed? Just thinking it's better (at least normally) to not have the same definition twice as it means if one changes one has to remember to change the others or else have some problems.

And yes I'm looking at working on these tools though not a great deal. I'll probably (not certain) do a pull request at some point today but I won't be doing a lot today either.

Probably the jnum_chk.h define should be used instead.

xexyl commented 2 years ago

Probably the jnum_chk.h define should be used instead.

Does that mean #include it or we don't need it for now so remove it?

lcn2 commented 2 years ago

Should formed_timestamp in test_formed_timestamp() be checked against someone using a value too far in the future? We do not think so.

An obvious for test_formed_timestamp() is to return false if the formed_timestamp was in the future. However, what if the system clock were off a bit? Consider a case where the IOCCC judges test a tarball that has just been uploaded by someone who is not trying to play games, except their system clock is off by a few minutes resulting in formed_timestamp > now.

In commit 71a6ad8ef0a3bcaf80da3181c8f00bb799b3afa9 we add MAX_CLOCK_ERROR:

#define MAX_CLOCK_ERROR ((42*60)-1)     /* maximum seconds allowed for a clock to be in error */

QUESTION: What is a good value for MAX_CLOCK_ERROR?

We don't know what a reasonable value of `MAX_CLOCK_ERROR` should be.   The current value is a guess and is subject to change before `IOCCCMOCK`.

We updated test_formed_timestamp() to return false if formed_timestamp > now + MAX_CLOCK_ERROR.

We fixed issues relating to time_t so that the code will work is time_t is signed or unsigned.

See commit 7b744c83f42c8659e798f13895ef8426bd2387f4

lcn2 commented 2 years ago

Probably the jnum_chk.h define should be used instead.

Does that mean #include it or we don't need it for now so remove it?

The value should be defined only in one place, and jnum_chk.h seems to be the a good place to define it.

xexyl commented 2 years ago

Probably the jnum_chk.h define should be used instead.

Does that mean #include it or we don't need it for now so remove it?

The value should be defined only in one place, and jnum_chk.h seems to be the a good place to define it.

Of course. But what I meant is do we right now need to include that header file in chkentry.h ?

xexyl commented 2 years ago

Should formed_timestamp in test_formed_timestamp() be checked against someone using a value too far in the future? We do not think so.

Define too far in the future. I guess one could say that if it's > (by a lot?) the closing time then maybe disqualify it. But then you bring up a valid point below and it's not a simple problem either.

An obvious for test_formed_timestamp() is to return false if the formed_timestamp was in the future. However, what if the system clock were off a bit? Consider a case where the IOCCC judges test a tarball that has just been uploaded by someone who is not trying to play games, except their system clock is off by a few minutes resulting in formed_timestamp > now.

I totally agree with this. One could maybe trigger a warning but not disqualify it ?

In commit 71a6ad8 we add MAX_CLOCK_ERROR:

#define MAX_CLOCK_ERROR ((42*60)-1)     /* maximum seconds allowed for a clock to be in error */

QUESTION: What is a good value for MAX_CLOCK_ERROR?

We don't know what a reasonable value of `MAX_CLOCK_ERROR` should be.   The current value is a guess and is subject to change before `IOCCCMOCK`.

I don't know either. Of course a clock drift or any problem with clock not being set right could cause any number of problems including possibly other invalid data being collected as well so that might also be something to consider?

We updated test_formed_timestamp() to return false if formed_timestamp > now + MAX_CLOCK_ERROR.

My thinking is the mock contest might be a good way to see how this works out. Perhaps (since nobody will win anyway) you could encourage people to submit entries with messed up time? (Maybe include instructions to do so?) Perhaps also given this you might (for the mock contest) increase the number of entries a user can submit so that more things can be tested. For example I had every intention of including one old entries of mine that did not win (and I am not surprised they did not win!) and one included actually would trigger one of the warnings (empty prog.c - it was a fun idea but I didn't expect it to win because of the brilliant entry from the 1990s). But this would trigger a warning so it would be a good entry to test.

We fixed issues relating to time_t so that the code will work is time_t is signed or unsigned.

Good idea.

See commit 7b744c8

Meanwhile I'm hoping to get some commits in soon. I am getting quite tired though so I'm hoping I can manage this. I might be better if I turn the fan to medium but my hands and feet get very cold so this is a problem (it's only on low but that might not be enough).

xexyl commented 2 years ago

...expect some commits and a pull request soon. I'll update this comment with a link after I've done so. I then will not do anything else here for the day most likely though I can probably interact a bit. Still I think I've made good progress. There is an XXX thing to fix but that requires future functions not yet written.

UPDATE 0:

See https://github.com/ioccc-src/mkiocccentry/pull/324/commits/a276c44e606bc7f2a470d0a6805904c88df3217c (there were some other commits too).

As for the concerns here's what I wrote in the log but which I include here so you're sure to see it (though you likely would see it in the file itself):

    There IS an issue to resolve but this must wait for other functions to
    exist which have not yet been written (and on my behalf I haven't even
    thought about it - yet). This is noted with a couple XXX comments.
    Specifically the function verify_entry_dir() is designed to prevent code
    duplication but at this time it would actually (if not modified) create
    code duplication.

    The function verifies that the correct files exist (if the entry_dir ==
    NULL then it's expected that .info.json and .author.json paths have been
    specified where a '.' means ignore that file). It does this (in the case
    of a directory being specified which is the single arg form of the
    command) by using chdir(2) (first recording the current working
    directory with open(2) with path ".") and then (if it succeeds) it runs
    the usual exists(), is_file() etc. functions on them. If the directory
    != NULL then we specifically set the filenames to ".info.json" and
    ".author.json"; if it's NULL the user will have specified the paths.

    Now as for the duplication the problem is that we will later have to
    chdir() into the directory in that form which will be a duplication of
    the code. There are at least two ways to solve this problem which I have
    noted. One is that there are functions that get the current working
    directory and then change into the other directory and one that also
    undoes these actions. Whether these belong in util.c or not I don't
    know. Another option I thought of is that we only check utility paths
    (or at this point single path - fnamchk) in the chkentry_sanity_chks()
    function and let the functions that process the files (or directory) do
    the checking. However this could also result (and I only just thought of
    this) code duplication depending on how it's done as one way to get the
    files is by directory and another is direct paths. But since the
    functions that actually validate the files have not been created yet
    these issues can be answered later.

I also noted the name might be problematic:

    One minor thing about this function is it might be a misnomer or
    misleading: it does not actually validate that the directory's
    .info.json and .author.json files are correct but rather that the files
    exist. Perhaps a better name could be devised but for now this will do.

Off to do something else now. Good day my friend!

lcn2 commented 2 years ago

Probably the jnum_chk.h define should be used instead.

Does that mean #include it or we don't need it for now so remove it?

The value should be defined only in one place, and jnum_chk.h seems to be the a good place to define it.

Of course. But what I meant is do we right now need to include that header file in chkentry.h ?

Include that header file in chkentry.h seems reasonable.

xexyl commented 2 years ago

Probably the jnum_chk.h define should be used instead.

Does that mean #include it or we don't need it for now so remove it?

The value should be defined only in one place, and jnum_chk.h seems to be the a good place to define it.

Of course. But what I meant is do we right now need to include that header file in chkentry.h ?

Include that header file in chkentry.h seems reasonable.

Actually that won't work so easily due to other symbols in that file:

In file included from chkentry.c:36:
./chkentry.h:69:27: error: redefinition of 'usage_msg'
static const char * const usage_msg =
                          ^
./jnum_chk.h:57:27: note: previous definition is here
static const char * const usage_msg =
                          ^
In file included from chkentry.c:36:
./chkentry.h:97:6: error: redefinition of 'quiet'
bool quiet = false;                             /* true ==> quiet mode */
     ^
./jnum_chk.h:89:6: note: previous definition is here
bool quiet = false;                             /* true ==> quiet mode */
     ^
2 errors generated.
make: *** [chkentry.o] Error 1

Which means that another include guard would have to be included. Alternatively it could go in the limits file maybe ?

Anyway I'll have to do it another time if you don't take care of it. I can do it but not until tomorrow as I'll be hading off shortly.

lcn2 commented 2 years ago

Probably the jnum_chk.h define should be used instead.

Does that mean #include it or we don't need it for now so remove it?

The value should be defined only in one place, and jnum_chk.h seems to be the a good place to define it.

Of course. But what I meant is do we right now need to include that header file in chkentry.h ?

Include that header file in chkentry.h seems reasonable.

Actually that won't work so easily due to other symbols in that file:

In file included from chkentry.c:36:
./chkentry.h:69:27: error: redefinition of 'usage_msg'
static const char * const usage_msg =
                          ^
./jnum_chk.h:57:27: note: previous definition is here
static const char * const usage_msg =
                          ^
In file included from chkentry.c:36:
./chkentry.h:97:6: error: redefinition of 'quiet'
bool quiet = false;                             /* true ==> quiet mode */
     ^
./jnum_chk.h:89:6: note: previous definition is here
bool quiet = false;                             /* true ==> quiet mode */
     ^
2 errors generated.
make: *** [chkentry.o] Error 1

Which means that another include guard would have to be included. Alternatively it could go in the limits file maybe ?

Anyway I'll have to do it another time if you don't take care of it. I can do it but not until tomorrow as I'll be hading off shortly.

See commit e881e27664bb92a70deffa5aeba18e7d0d621fbf

Moved MATCH_PRECISION to util.h.

UPDATE 0

So it seems that "Include that header file in chkentry.h" was not reasonable. :-)

xexyl commented 2 years ago

Which means that another include guard would have to be included. Alternatively it could go in the limits file maybe ? Anyway I'll have to do it another time if you don't take care of it. I can do it but not until tomorrow as I'll be hading off shortly.

See commit e881e27

Moved MATCH_PRECISION to util.h.

That makes sense too yes.

UPDATE 0

So it seems that "Include that header file in chkentry.h" was not reasonable. :-)

Indeed.

xexyl commented 2 years ago

Before I go:

I gave more ideas on how to solve the possible code duplication in that pull request. However right now there actually is not a code duplication and there won't be unless when those functions that validate the files are added it's not addressed at that point. I will happily work on those functions too but right now I cannot as I need to get going for the day. It would be easy to solve the non-existent problem that would become a problem if not addressed.

On the other hand the name for it might be not the best but that can be fixed later - or else it can do both - validate that the files exist as well as check the files. In fact that might be a good way to go about it:

Verify that each file exists and then for each one that does exist it would run the right checks on it. It could be stub functions for now. That would mean that very little code would have to be moved. If you want I can do that. If you merge I can update it anew but otherwise I can just do more of it tomorrow.

xexyl commented 2 years ago

I just made some stub functions that should help you out. If the files are valid json we will have a struct json * which can then be used. Separated the call to that function from the sanity checks function.

See the diff and the log:

    This commit divorces the verify_entry_dir (now called
    validate_entry_files()) from chkentry_sanity_chks(). If
    chkentry_sanity_chks() does not pass the validate_entry_files() will not
    run. If it does pass it will run and depending on what args are used and
    what is readable it will run (for now only stub) functions to validate
    the respective files.

    What this means is that for now if the file is invalid json the program
    will return exit code of non-zero. If the json is valid it will be 0.
    This addresses the concern of possible code duplication.

Must go now but I can fix more tomorrow if necessary. Good day!

UPDATE

Real quick. Here are some example outputs:

$ ./chkentry -J 2 test_work/test-0/
in parse_json(): JSON debug[1]: valid JSON
in parse_json(): JSON debug[1]: valid JSON
$ ./chkentry -J 9 zz zz
in parse_json_file(): JSON debug[5]: Calling parse_json with length 4:
*** BEGIN PARSE
<
{\0}\n
>
Warning: ugly_lex: 
invalid token: 0x00 = <>
syntax error line: 1: empty text
*** END PARSE
in parse_json(): JSON debug[9]: ugly_parse() returned: 1
in parse_json(): JSON debug[1]: invalid JSON
in parse_json_file(): JSON debug[5]: Calling parse_json with length 4:
*** BEGIN PARSE
<
{\0}\n
>
Warning: ugly_lex: 
invalid token: 0x00 = <>
syntax error line: 1: empty text
*** END PARSE
in parse_json(): JSON debug[9]: ugly_parse() returned: 1
in parse_json(): JSON debug[1]: invalid JSON
in parse_json_file(): JSON debug[5]: Calling parse_json with length 4:
*** BEGIN PARSE
<
{\0}\n
>
Warning: ugly_lex: 
invalid token: 0x00 = <>
syntax error line: 1: empty text
*** END PARSE
in parse_json(): JSON debug[9]: ugly_parse() returned: 1
in parse_json(): JSON debug[1]: invalid JSON
in parse_json_file(): JSON debug[5]: Calling parse_json with length 4:
*** BEGIN PARSE
<
{\0}\n
>
Warning: ugly_lex: 
invalid token: 0x00 = <>
syntax error line: 1: empty text
*** END PARSE
in parse_json(): JSON debug[9]: ugly_parse() returned: 1
in parse_json(): JSON debug[1]: invalid JSON
$ ./chkentry zz test_work/test-0/.author.json 
Warning: ugly_lex: 
invalid token: 0x00 = <>
syntax error line: 1: empty text
Warning: ugly_lex: 
invalid token: 0x00 = <>
syntax error line: 1: empty text

The error message of course refer to the file zz and this makes me think of the issue that we need to have the filename reported as well. But as you can see when the file is valid json we have a proper struct and if not it'll be an error.

Okay one more example:

$ ./chkentry test_work/test-0/{.info.json,.author.json}
$ echo $?
0

Well I'll be going now. If you want anything corrected I'll do it tomorrow.

xexyl commented 2 years ago

Leaving for now but wanted to say I fixed a bug that I thought about last night that actually broke make test.

Other than that I'll hold off on chkentry until you have decided the direction you need it to go as per comments https://github.com/ioccc-src/mkiocccentry/pull/324#issuecomment-1232364492 and https://github.com/ioccc-src/mkiocccentry/pull/324#issuecomment-1232392182.

Please advise when you have sorted out the thoughts in your head and (if necessary) put them on screen (as opposed to paper).

We can talk about it of course and we can discuss other things too but I'll hold off on modifying the code. Leaving now for a while ....

lcn2 commented 2 years ago

We plan to do some major surgery on recent pull requests involving chkentry when it comes to integrating JSON semantic testing. However, before we do that surgery, we need to reach code complete on the JSON semantic code itself BEFORE we start whacking code in chkentry.c and chkentry.h.

We mention this, @xexyl, because you might not want to put in much more effort into adding code to chkentry (in light of the above mentioned future actions) and instead focus on other repo issues.

However, we are also engage with our family in regards to a wedding of a relative and are currently in a region where cell phone service is poor AND local hotels have a network that tends to disconnect VPNs about 2 minutes after they are started. So edits are limited to occasions where we have more reliable VPN service and in more complex cases, where it is socially acceptable to break out a laptop and work.

This is all to say, our process for the next several days may be limited at best.

xexyl commented 2 years ago

We plan to do some major surgery on recent pull requests involving chkentry when it comes to integrating JSON semantic testing. However, before we do that surgery, we need to reach code complete on the JSON semantic code itself BEFORE we start whacking code in chkentry.c and chkentry.h.

We mention this, @xexyl, because you might not want to put in much more effort into adding code to chkentry (in light of the above mentioned future actions) and instead focus on other repo issues.

Yes. I've not touched it since you told me that it's not fully thought out yet and that the design/flow of it is not certain.

However, we are also engage with our family in regards to a wedding of a relative and are currently in a region where cell phone service is poor AND local hotels have a network that tends to disconnect VPNs about 2 minutes after they are started. So edits are limited to occasions where we have more reliable VPN service and in more complex cases, where it is socially acceptable to break out a laptop and work.

Good luck with all that!

This is all to say, our process for the next several days may be limited at best.

No problem. I'll keep busy where I can when I can.

Good day!

xexyl commented 2 years ago
#define If you get here you are not as lazy as the C pre-processor which just uses this line for no good purpose

Good one! I just noticed it. Thanks for the laugh! Much better than the comment I had.

xexyl commented 2 years ago

Btw I meant to mention that I love this comment:

 *     --  Sirius Cybernetics Corporation Complaints Division, JSON spec department. :-)

Well done!

lcn2 commented 2 years ago
```c
#define If you get here you are not as lazy as the C pre-processor which just uses this line for no good purpose

Good one! I just noticed it. Thanks for the laugh! Much better than the comment I had.

Thanks .. we had something like this in mind but waited to apply such "foo fun" UNTIL AFTER we completed a milestone (as noted in v0.6 CHANGES.md).

We might try some improvements once we reach v0.7 (when JSON semantic test and chkentry is complete) .. until then we need to remember that people are waiting for the next IOCCC and we need to run IOCCCMOCK in order to learn from our mistakes before the next real IOCCC is opened. As such, time to refocus less on "fun" a more on "fun-ctions" :-)

UPDATE 0

At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

lcn2 commented 2 years ago

Btw I meant to mention that I love this comment:

 *     --  Sirius Cybernetics Corporation Complaints Division, JSON spec department. :-)

Well done!

Thanks. We think the JSON people might see the humor while maintaining a tad bit of technical criticism. :-)

xexyl commented 2 years ago
```c
```c
#define If you get here you are not as lazy as the C pre-processor which just uses this line for no good purpose

Good one! I just noticed it. Thanks for the laugh! Much better than the comment I had.

Thanks .. we had something like this in mind but waited to apply such "foo fun" UNTIL AFTER we completed a milestone (as noted in v0.6 CHANGES.md).

Yes.

We might try some improvements once we reach v0.7 (when JSON semantic test and chkentry is complete) .. until then we need to remember that people are waiting for the next IOCCC and we need to run IOCCCMOCK in order to learn from our mistakes before the next real IOCCC is opened. As such, time to refocus less on "fun" a more on "fun-ctions" :-)

True on all parts. Even so it doesn't take any time away for me to add strings usually as they are typically already things I've noted / seen / can copy paste etc. so I do that with other commits. I think you should come up with a list of quotes you'd like to add too so that once you have the tool you can but of course you can add them at the end if you so wish.

xexyl commented 2 years ago

UPDATE 0 At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

The solution is either a space (if that would work for twitter?) or else a screenshot. But the questions are: what do I include with it ? And should I do @ioccc ? Also should I include any other lines ?

xexyl commented 2 years ago

Btw I meant to mention that I love this comment:

 *     --  Sirius Cybernetics Corporation Complaints Division, JSON spec department. :-)

Well done!

Thanks. We think the JSON people might see the humor while maintaining a tad bit of technical criticism. :-)

Well I'm not sure. They're more likely to see the humour than what I did anyway! I still think it's a funny expansion of JSON and the commentary was great. I mean yes it was kind of mean but it was good satire for the IOCCC. Even so it was going too far I guess.

This does remind me of a meme I made not too long ago. Cybernetics I mean. I'll see about posting it in the more OT thread.

xexyl commented 2 years ago

UPDATE 0 At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

The solution is either a space (if that would work for twitter?) or else a screenshot. But the questions are: what do I include with it ? And should I do @ioccc ? Also should I include any other lines ?

I see you reacted with a thumbs up but have not answered the parts to it. I'd be happy to do it in the coming days if you have some thoughts - since it was your idea I presume you might have been thinking of something specific?

lcn2 commented 2 years ago

UPDATE 0 At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

The solution is either a space (if that would work for twitter?) or else a screenshot. But the questions are: what do I include with it ? And should I do @ioccc ? Also should I include any other lines ?

I see you reacted with a thumbs up but have not answered the parts to it. I'd be happy to do it in the coming days if you have some thoughts - since it was your idea I presume you might have been thinking of something specific?

If you have an way to improve the #define, lets consider it LATER .. once we make more progress towards IOCCCMOCK.

xexyl commented 2 years ago

UPDATE 0 At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

The solution is either a space (if that would work for twitter?) or else a screenshot. But the questions are: what do I include with it ? And should I do @ioccc ? Also should I include any other lines ?

I see you reacted with a thumbs up but have not answered the parts to it. I'd be happy to do it in the coming days if you have some thoughts - since it was your idea I presume you might have been thinking of something specific?

If you have an way to improve the #define, lets consider it LATER .. once we make more progress towards IOCCCMOCK.

After? Sure. That makes sense. But what about the other parts wrt twitter ? You know what. I will make a reminder for one month from now which hopefully will be a good timeframe. That way we can do it but we can focus on other things.

I'll be doing a commit soonish I think: updates to two man pages (adding NOTES, removing AUTHORS and in one of them FILES - the other already had that section removed). Still not sure how soon it'll be as I'm quite distracted with other things ... looking at my personal fortune file and other things. Unfortunately my faculties atm are not very good to do much thinking and I believe that after the commit I'll be doing something else.

xexyl commented 2 years ago

UPDATE 0 At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

The solution is either a space (if that would work for twitter?) or else a screenshot. But the questions are: what do I include with it ? And should I do @ioccc ? Also should I include any other lines ?

I see you reacted with a thumbs up but have not answered the parts to it. I'd be happy to do it in the coming days if you have some thoughts - since it was your idea I presume you might have been thinking of something specific?

If you have an way to improve the #define, lets consider it LATER .. once we make more progress towards IOCCCMOCK.

Well one possibility just popped into my head ... I wasn't actually thinking about it though. It just popped into my head. I don't know if this is actually legal C but I believe it might be because without the # it doesn't change the meaning anyway:

#define undef if (you get here) you are not as lazy as the C pre-processor which just uses this line for no good purpose;

So if one were to use after it #undef it would require a macro name but undef would result in a compilation error. Is this valid? But even if it's not we could just do:

#define if (you get here) you are not as lazy as the C pre-processor which just uses this line for no good purpose;

or make the name foo like:

#define foo if (you_get_here) you_are_not_as_lazy_as_the_C_pre_processor_which_just_uses_this_line_for_no_good_purpose();

Anyway even that's not something that I am going to think about today beyond this. I'm going to do something else entirely - though I am thinking of watching that video that Leo referred us both to. Not sure though. A documentary series I have watched before is also something I've been considering watching again lately (would be the third or maybe fourth time ... it's a good one on the Troubles done by the BBC in 2019).

xexyl commented 2 years ago

UPDATE 0 At the risk of contriducting the above, if you have a way to tweet the #define, feel free to consider .. perhaps as a reward for completing another issue for this repo. :-)

The solution is either a space (if that would work for twitter?) or else a screenshot. But the questions are: what do I include with it ? And should I do @ioccc ? Also should I include any other lines ?

I see you reacted with a thumbs up but have not answered the parts to it. I'd be happy to do it in the coming days if you have some thoughts - since it was your idea I presume you might have been thinking of something specific?

If you have an way to improve the #define, lets consider it LATER .. once we make more progress towards IOCCCMOCK.

Well one possibility just popped into my head ... I wasn't actually thinking about it though. It just popped into my head. I don't know if this is actually legal C but I believe it might be because without the # it doesn't change the meaning anyway:

#define undef if (you get here) you are not as lazy as the C pre-processor which just uses this line for no good purpose;

So if one were to use after it #undef it would require a macro name but undef would result in a compilation error. Is this valid? But even if it's not we could just do:

#define if (you get here) you are not as lazy as the C pre-processor which just uses this line for no good purpose;

or make the name foo like:

#define foo if (you_get_here) you_are_not_as_lazy_as_the_C_pre_processor_which_just_uses_this_line_for_no_good_purpose();

Even though I'm still curious about my question, whether:

#define define foo`

is technically legal (I think it is but I'm not certain) I have come up with a great way to do this. It just popped into my head. I think you'll like it. I might put it in the code but for now I have it in my tool as I have a copy of that function anyway for testing purposes in the tool's source file.

lcn2 commented 2 years ago

Even though I'm still curious about my question, whether:

define define foo`

is technically legal (I think it is but I'm not certain) I have come up with a great way to do this.

It is possibly dangerous. We replaced you're in the #define of the last line of foo.c for that reason.

Let's leave foo fun alone while we focus on making progress on IOCCCMOCK.

We make good progress nearing code complete for waking the JSON parse tree while processing a semantic table. However it may be a day+ before we get to pushing out the code AND we may want to have the chkentry code start to drive such code before we start to push it out.

xexyl commented 2 years ago

Even though I'm still curious about my question, whether:

define define foo`

is technically legal (I think it is but I'm not certain) I have come up with a great way to do this.

It is possibly dangerous. We replaced you're in the #define of the last line of foo.c for that reason.

I'm confused on the second sentence though. I didn't have 'you're' there ? I did have a comment originally but you changed it to a define. Did you accidentally have 'you're' in it or do you mean that when changing it to a define you changed it to e.g. you are?

Let's leave foo fun alone while we focus on making progress on IOCCCMOCK.

Yes of course. Mind you I already thought of an improvement to the above and it doesn't have the above issue.

I should point out that these things just pop into my head sometimes even when I'm not at the computer to be able to work on say chkentry! So just because I am playing with this does not mean I'm taking away time from the more important thing of the mock contest. I hope that clarifies things for you there.

We make good progress nearing code complete for waking the JSON parse tree while processing a semantic table. However it may be a day+ before we get to pushing out the code AND we may want to have the chkentry code start to drive such code before we start to push it out.

I was thinking the same thing - that good progress is being made. I'm happy about that. Looking forward to seeing what you have in mind. Once you have decided the way chkentry needs to work I'll be happy to work on it too: been holding off since you said it's not clear yet what you need.

xexyl commented 2 years ago

Even though I'm still curious about my question, whether:

define define foo`

is technically legal (I think it is but I'm not certain) I have come up with a great way to do this.

It is possibly dangerous. We replaced you're in the #define of the last line of foo.c for that reason.

I'm confused on the second sentence though. I didn't have 'you're' there ? I did have a comment originally but you changed it to a define. Did you accidentally have 'you're' in it or do you mean that when changing it to a define you changed it to e.g. you are?

Actually I don't have any ' (or it looks like that was a backtick) in the macro at all. I would only do the former when I need say the literal constant value like 'a' or something like that (and the latter never). I was more thinking of this:

#define define

That won't stop #define from working but will it end up allowing define by itself to possibly be used? I would think that if it did it would be undefined and that's what I was after. Even so I don't have it that way now. It's fun what I thought of but I'm not worrying about that for now - due to the reasons you cited.

EDIT

Ah.. I see. There was a typo on my part. Remove the erroneous character. It should have only had isalpha() chars.

xexyl commented 2 years ago

Re commit 2db7f241bbfbcfda32fb5b46f769f92dce92dcc2:

    Remove old.* files

    We copied the old.* files into a local private directory, just is
    case some of that code is needed as we complete the JSON semantical
    tests.  We will remove that private directory once we are finished.

I did too. I probably won't end up getting rid of them though at least for a while. In any event there is one thing that might be good to have and that's the ability to test for specific errors so that we know that a file that fails fails for the correct reasons. But I know you'll work that out too.

I'll be leaving soon for the day. Hope you have a good night.

OT

Oh and btw for quite some time now I've been hearing a call (to make a reference to something in the book I'm about to mention) - to read The Lord of the Rings again. I don't know if I have the energy to do so but it's been something I've been thinking about for some while. Usually by now in the year I would have already read it at least once or twice. Still though it probably won't prevent me from working on the repo either way.

Have a great night and sleep well when you do.

lcn2 commented 1 year ago

Commits 81b6fcbf940e8cb5e8a44cfb06d8fbc9e6d7ba78, c854c30459a933001df2f3849c22d0efe28a367d, and fd43e6553043f38666beebd95f5dd1ae4dab1445 were needed to make is much easier to JSON parse an open stream.

We need to completely re-working the interface for chkentry.c in order to separate directory and file processing from JSON parsing that in turn produce a JSON parse tree. Once we have both JSON parse trees, then we will perform JSON semantic tests and then report on any errors that we might have encountered.

This use case is a good test case for the JSON parser as well as a test for sequential JSON parsing.

Stay tuned.

xexyl commented 1 year ago

Commits 81b6fcb, c854c30, and fd43e65 were needed to make is much easier to JSON parse an open stream.

Looks good. I've been holding off on this tool until you had a better idea of what you really want. The changes I had made were I think a best guess but I look forward to hearing more about your ideas! I can then help work on it.

We need to completely re-working the interface for chkentry.c in order to separate directory and file processing from JSON parsing that in turn produce a JSON parse tree. Once we have both JSON parse trees, then we will perform JSON semantic tests and then report on any errors that we might have encountered.

Well as above once you have a clearer idea I'll be happy to help!

This use case is a good test case for the JSON parser as well as a test for sequential JSON parsing.

Are you saying that this will be a great example for when I add documentation to jparse ?

Stay tuned.

As above I certainly will and look forward to it!

lcn2 commented 1 year ago

Are you saying that this will be a great example for when I add documentation to jparse ?

Yes.

xexyl commented 1 year ago

Are you saying that this will be a great example for when I add documentation to jparse ?

Yes.

Sounds good. I actually took a very brief look at it after writing that and I can see it as well.

xexyl commented 1 year ago

It's funny: commit f3a0ef9f with recording the cwd: I almost did that yesterday too but I didn't which direction you're needing it so I didn't do it. Then you ended up doing it anyway! This is not the first time this has happened of course.

Well as the saying goes: two great minds think alike. I have my own quote about this:

Two great deluded minds think alike -- for each other.

This is in reference to shared psychotic disorder. In Germany it's also a funny saying:

Zwei Doofe ein Gedanke

It's supposed to mean 'two minds think alike' but it actually translates to:

Two fools one thought.

Hope you're sleeping well!

lcn2 commented 1 year ago

Hope you're sleeping well!

Slept well .. hope for same for you later tonight.

xexyl commented 1 year ago

Hope you're sleeping well!

Slept well .. hope for same for you later tonight.

I'm happy you did sleep well! No luck for me .. same again. It's getting absurd waking up so early. But nothing I can do about it :(

Anyway I see with commit 9c94ad7dd07590013685cbc35fb5734ebd14f2f4 that you added some code / fixed a bug with chkentry. I just wanted to say again that once you have a clearer idea of what needs to be done with this tool I'm happy to help.

As for me I am wanting to look at bug_report.sh next but whether I can manage or not I don't know. I had some thoughts on how to proceed last night. Unfortunately I fear I might have to go afk soon but if so I hope to continue this when I get back. I don't think there's a chance I will fall asleep again :( but hopefully I can then at least do some things. Good thing is (I guess) I went to sleep earlier so I did get more sleep and that's exactly why I went to sleep earlier. I woke up about the same time as yesterday (maybe a 10 - 15 minutes difference) but I went to sleep an hour earlier so I did get more sleep.

As for sleep I do hope you're sleeping well my friend! Anyway I just wanted to remind you that once you're clearer on what you need with chkentry I'm happy to help!

lcn2 commented 1 year ago

Birthday checkpoint v62.0 ... or is it v6.2? :-)

See commit ad5e8703839b902c98084778866f6a9fc6e65366

xexyl commented 1 year ago

Re commit ba5623b93463e13498c735a5e948ae1803e3ba77 where chkentry is complete: that's GREAT news! I'll change the header comments to say it was developed rather than being developed by us. Not just yet though. Looking at some other things first ...

Hope you're sleeping well my friend!

xexyl commented 1 year ago

Re commit ba5623b where chkentry is complete: that's GREAT news! I'll change the header comments to say it was developed rather than being developed by us. Not just yet though. Looking at some other things first ...

Hope you're sleeping well my friend!

See https://github.com/ioccc-src/mkiocccentry/pull/382/commits/ea6407583b002ad51401b3c6a1ca797c995ac351:

Fix mkiocccentry_test.sh and update chkentry

The zero length rule 2a and rule 2b sizes will no longer return false.
However I did note in a comment (in both functions) that currently the
override booleans are not checked and my guess is this needs to be
added. However as I'm not as clear on the json semantics testing I've
not done this. Nevertheless chkentry now passes with 0 rule_2a_size and
rule_2b_size so mkiocccentry_test.sh now passes. Besides the override
check I believe this makes chkentry entirely complete.

I've updated the comments for chkentry as well.

The man page was updated to no longer say it's not completed.

Please note the note :-) (which I also noted in the pull request comment)

xexyl commented 1 year ago

With commit 029b5ff8a99d167f34b18f083bfef75f02a2d603 this tool now has a test suite. As I noted though if there is some other requirement that the script does not fulfil I will be happy to have another script or modify it. I know you were going to work on something but this felt very similar to the jparse_test.sh so it was pretty easy to do. The man page was also added.

Do we want a way to test that the errors given for the bad files are those expected? If so we might want to look at the old check files?

Another point is there seem to be some files that are actually bad test cases as you'll notice that the script exits 1. That's one reason to not have it in icccc_test.sh for now.

I'm going to probably take a break now but hopefully this will save you some time! But like I said I'm happy to remove that file or change it or have you reject that commit specifically (I think you can merge individual commits?). Whatever you need I'll do it.

EDIT 0

BTW: since some of the commits changed (deleted) a lot of invalid files it would be better that if you cannot merge individual commits that I just change the script or make it empty (a stub or whatever) so that you can then edit it. That way I can not worry about having to get the commit rights. I hope that that's okay but if not I'll work it out. I have a way to do it but it would be easier otherwise.

Break time but I feel good about what I did this morning!

EDIT 1

One of the problems at least is that something was changed after the original creation of the file. For example missing no comment in the files.

xexyl commented 1 year ago

Correct me if I'm wrong but the following file are in error:

If any of these are not true then we have a problem. If they are true we can delete them or move them to the good subdirectory.

Additionally this should probably be checked for as I noted a git log today:

./test_JSON/info.json/bad/info.NUL.json tests out okay because NUL bytes in strings are valid JSON but we don't check for it in the string related checks.

However: ./test_JSON/info.json/bad/info.manifest-28.json and ./test_JSON/info.json/bad/info.manifest-29.json should actually fail because the manifest has two Makefiles and two files called extra2 (case insensitive), respectively.

For the first one: one is in the Makefile field (or whatever the term is in json) and the other is called makefile in an extra file. Similar is done for the other manifest file: same name but different case.

I believe the function object2manifest is in error because it checks the name for these but not the value. Additionally it should be a case-insensitive check in some cases but not all (because it has to be .info.json and Makefile and so on in the correct fields/whatever).

I am however too tired to look at this in more detail for now. Maybe I can look at it later on.

I really am going for a break now!

UPDATE 0

With commit 6beb0838608a84e39ab8f1a1a3866b8fbcb9ebbe the test for test_JSON/info.json/bad/info.manifest-28.json now passes (i.e. chkentry rejects it). The other manifest one still fails the test.

UPDATE 1

With commit c86e577eb71d5e4fbe1caa2da6c0a4b0baa8c86b the test for test_JSON/info.json/bad/info.manifest-29.json also now passes (chkentry rejects it) with the same caveat as the below comment.

xexyl commented 1 year ago

UPDATE 0

With commit 6beb083 the test for test_JSON/info.json/bad/info.manifest-28.json now passes (i.e. chkentry rejects it). The other manifest one still fails the test.

I should point out though that it's only in one location. I noticed that in some places the count is incremented but that's not done here as I didn't address that part (at first I thought I was but it didn't seem right so I undid that change). I simply made test_extra_file do case insensitive compare just like you had me do in txzchk. If the count should also be incremented perhaps you'd like to do that? Still the file now is rejected on the grounds of one of the extra files is a required file.

Unless the case insensitive check requirement has changed maybe?

UPDATE 0

See update 1 in https://github.com/ioccc-src/mkiocccentry/issues/259#issuecomment-1296944625.

If this is okay then the only other ones that are failing are the ones I listed in the other comment which might or might not be correct. If the test files are incorrect then they should be deleted or moved over to the good subdirectory. If they're correct we have another problem but I'm not sure what they are. That is why we had the check codes earlier on of course.

xexyl commented 1 year ago

Question for you.

Is there a reason in the script in the block:

if [[ -n "$PATCH_FILE" ]]; then

you only set trap to delete files if verbosity level is >= 3? I refer to:

    if [[ $V_FLAG -ge 3 ]]; then
        trap "exit" 0 1 2 3 15
    else
        trap "rm -f \$TMP_FILE \$TMP_FILE.rej \$TMP_FILE.orig; exit" 0 1 2 3 15
    fi

Shouldn't the files always be removed on exit?

And on that note I still don't know how well it works. I have for example:

$ ls
.hostchk.prog.Fpp0ZmxJgI* .hostchk.prog.Vvxvj0JQSF*

I can only think that maybe during an invocation of make test I interrupted it but I would think that trapping SIGINT would fix that. Not sure. I know I added at the end of the script deletion of the temporary files (or in this case the end of that test) but it seems like there's a timing issue. And perhaps that's just it - the race that there is no real good solution for in all cases.

Anyway why only trap to delete those files if verbosity is >= 3 ?

lcn2 commented 1 year ago

You are working in a very active area .. for which we probably either have to toss our edits or toss your edits or try to combine them.

As we stated in multiple TODOs, our next focus area is in testing.

Question for you.

Is there a reason in the script in the block:

if [[ -n "$PATCH_FILE" ]]; then

you only set trap to delete files if verbosity level is >= 3? I refer to:

    if [[ $V_FLAG -ge 3 ]]; then
        trap "exit" 0 1 2 3 15
    else
        trap "rm -f \$TMP_FILE \$TMP_FILE.rej \$TMP_FILE.orig; exit" 0 1 2 3 15
    fi

Shouldn't the files always be removed on exit?

And on that note I still don't know how well it works. I have for example:

$ ls
.hostchk.prog.Fpp0ZmxJgI* .hostchk.prog.Vvxvj0JQSF*

I can only think that maybe during an invocation of make test I interrupted it but I would think that trapping SIGINT would fix that. Not sure. I know I added at the end of the script deletion of the temporary files (or in this case the end of that test) but it seems like there's a timing issue. And perhaps that's just it - the race that there is no real good solution for in all cases.

Anyway why only trap to delete those files if verbosity is >= 3 ?

Because when debugging deep enough, we wanted to avoid removing the temporary files related to patching.

UPDATE 0

We are going to bypass all of those questions in order to get ahold of the testing we need for chkentery(1).

xexyl commented 1 year ago

You are working in a very active area .. for which we probably either have to toss our edits or toss your edits or try to combine them.

As we stated in multiple TODOs, our next focus area is in testing.

Question for you. Is there a reason in the script in the block:

if [[ -n "$PATCH_FILE" ]]; then

you only set trap to delete files if verbosity level is >= 3? I refer to:

    if [[ $V_FLAG -ge 3 ]]; then
        trap "exit" 0 1 2 3 15
    else
        trap "rm -f \$TMP_FILE \$TMP_FILE.rej \$TMP_FILE.orig; exit" 0 1 2 3 15
    fi

Shouldn't the files always be removed on exit? And on that note I still don't know how well it works. I have for example:

$ ls
.hostchk.prog.Fpp0ZmxJgI* .hostchk.prog.Vvxvj0JQSF*

I can only think that maybe during an invocation of make test I interrupted it but I would think that trapping SIGINT would fix that. Not sure. I know I added at the end of the script deletion of the temporary files (or in this case the end of that test) but it seems like there's a timing issue. And perhaps that's just it - the race that there is no real good solution for in all cases. Anyway why only trap to delete those files if verbosity is >= 3 ?

Because when debugging deep enough, we wanted to avoid removing the temporary files related to patching.

That's all I could think of. Just wanted to be sure.