-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhancement: Improve chkentry(1) to check the JSON semantics of a winning IOCCC entry #940
Comments
Coped from GH-issuecomment-2267089148:
We don't want to imply that if We suggested a change in behavior on the To process an entry ./chkentry YYYY/dir For example: ./chkentry 2020/ferguson1 To check just ./chkentry some/path/.info.json . To check just ./chkentry . some/path/.auth.json To check ./chkentry some/path/.info.json some/path/.auth.json To have "undocumented fun" 🤩: ./chkentry . . Remember that a submission directory (with Remember that a submission directory could have The The If the The ./chkentry /tmp/curds /var/tmp/whey If the There will be a tool (such as The above line is one reason why we suggest that by default, the So this is why we suggest these command line forms: ./chkentry some/path/.info.json .
./chkentry . some/path/.auth.json
./chkentry some/path/.info.json some/path/.auth.json
./chkentry /tmp/curds /var/tmp/whey
./chkentry YYYY/dir This is why we are suggesting that a "one arg" This is why we are suggesting that a "two arg" |
Copied from GH-issuecomment-2267092165:
You are probably correct. The change to
will require and arg of the form |
Copied from GH-issuecomment-2267616067:
The last time there was a chance, we hand edited the patch. That's not good and so needs to change so that a known reference can be used as a model. Currently one can be generated based on a reference file and that result is patched so that some things such as the allows count range min and max are adjusted. See https://github.com/ioccc-src/mkiocccentry/blob/master/soup/chk.auth.ptch.c That might have been fine for development, but it is a bit of pain should a format need to change. So there might need to be a better was to specify the range count (instead of the current |
Copied from GH-issuecomment-2267623067:
One should be careful when one "over specifies" something: In this case adding a count of authors instead of just counting the authors. Over specification of something allows for multiple ways for something to be wrong and adds to the number of checks one has to make. So adding or removing and author from the set of authors would be complicated by now also having to adjust the author count number. And then the length of the list of authors now has to be compared with the author count. Adding a count of authors doesn't provide and missing information and create opportunities for something to be wrong. Yes, when forming Here, with It is a similar reason why the manifest in the But you might say: "how do we know if the list of authors is incomplete?" The answer is we let the humans decide. If a winner were posted with an author missing or incorrectly added to an entry, we would count on the humans noticing the problem and correcting it with a pull request. Adding an author count to the JSON wouldn't help detect a missing or extra author to an entry because the overall author count could also be wrong. #The same way we don't detect if a So in general, over specifying information is not helpful. It creates opportunities for redundant information to disagree with itself, requires even more consistency checks, complicates the overall JSON structure, and doesn't really help detect when content such as an author is missing or extra. |
Copied form GH-issuecomment-2267639223:
If the manifest of a While a weeeeeee tiny bit near the edge of semantic checking, if the actual file is missing in the directory, of if it the directory has a file not listed in the manifest, that should be flagged as an error too. |
Copied from GH-issuecomment-2267663149:
The need for this can be argued this way: the semantics of a field manifest depends, in part, on matching the files under the Assume that FYI: The tool that the judges use to help convert a submission into an entry will use It probably would hurt
This will catch the case where the judges, when converting a submission on a winning entry, forget to remove those 3 files. |
Copied from GH-issuecomment-2269551230:
If by that you ask to have the code, with lots of comments explained in GitHub comments? While we don't think you mean that, we just want to be sure you don't as the effort need to re-explain code comments and algorithms in GitHub comments would be larger than the effort to just write the tool. In terms of requirements: the single argument If you walk thru a file such as 1984/mullender/.entry.json you will see a number of JSON members. The code needs to determine if each of those JSON members are present, have the proper JSON member value type and in some cases the value is reasonable. The We suggest you approach this from a testing perspective. How might someone try to mess up a We won't go into every JSON member element in GitHub comments. You will have to figure that out yourself. In some cases you may even need to add a BTW: The single argument Think of a ways to mess up a Some high level requrementsWhen the single argument When a mistake is made in the contests of the By "mistakes made" we include things such as the wrong type of JSON value, or a JSON value that is the correct type but whose value is not proper, out of range, unreasonable, or is inconsistent with the IOCCC entry it is within, etc. When someone modifies an IOCCC entry but fails to update the contests of the And by detected we mean warnings or errors are issued on stderr and the tool does not exit with a 0 exit code. UPDATE 0aThink of ways that a |
Copied from GH-issuecomment-2269571888:
The patching system was, we admit, a but of a "bad hack" that might be showing its limits when the code has to be extended to checks for UPDATE 0If the result is that the requirements in GH-issuecomment-2269551230 are met, AND the tool is able to still do sanity checks for the submission |
Copied from GH-issuecomment-2272414067:
Hmm .. sure? But perhaps a higher level question needs to be consider instead. While one can generate a table for a given JSON file using jsemtblgen 2020/endoh1/.entry.json other semantically valid jsemtblgen 1984/anonymous/.entry.json How can you find the range of something? We hacked together what we thought of as a "bare minimum" JSON file for some types of JSON files. For example, for the jsemtblgen test_ioccc/test_JSON/info.json/good/info.min-manifest.json With the assumption that other somatically valid QUESTIONAre jsemtblgen generated tables the wrong approach? Perhaps jsemtblgen generated tables is the WRONG approach. What is needed, so to detect that those required JSON items be present, that they have the requires types, and that one is able to determine if the number of a given JSON item is OK or not. Take the We MUST have 1 and only 1 " We MUST have 1 and only 1 " We must have 1 and only 1 " We may have 0 or more " etc. All of the above requirements are BEFORE one calls upon validation functions such as Is there a BETTER way to verify what we call the semantics of a given type of JSON file? |
Copied from GH-issuecomment-2272443630:ProblemIt is rather difficult to look for questions within a long comment, then guess if something with a "?" is a question that needs to be answered or is later on rendered not important (because perhaps you found the answer, or you decided it was not important, or you were just pondering something out loud, or) and then to try and determine the context for the question, and the try to reply to the question. SuggestionWhen you find something you need to ask, put a Possible Answer to a previous questionWe think in GH-issuecomment-2271322924 you asked a question you wish us to answer. We will now try to quote the relevant text below:
OK, now we will make a guess that NAMED member might relate, given GH-issuecomment-2272396477, to the So now we will make a guess that "where there is a difference" might be asking about something where
is not a question since you seemed you have provided an example above it Maybe the real question lies in the that that immediately follows:
Here we will guess that (assuming our guess of name related to the Well the patch file --- ref/info.reference.json.c 2024-05-19 00:08:13
+++ chk_sem_info.c 2024-05-18 23:59:02
@@ -39,17 +39,17 @@
struct json_sem sem_info[SEM_INFO_LEN+1] = {
/* depth type min max count index name_len validate name */
- { 5, JTYPE_STRING, 1, 84, 84, 0, 0, NULL, NULL },
+ { 5, JTYPE_STRING, 10, 84, 84, 0, 0, NULL, NULL }, So there is a case where the That example is at level 5 of the JSON parse tree (where 0 is the root). Looking a minimal example of a .info.json file we see that at level 5 of the JSON tree we find 5 JSON strings that are NOT JSON member names: {"info_JSON" : ".info.json"},
{"auth_JSON" : ".auth.json"},
{"c_src" : "prog.c"},
{"Makefile" : "Makefile"},
{"remarks" : "remarks.md"} In particular the 5 JSON member value strings:
are those 5 JSON strings that are not JSON member names and, if you count the levels, are at the JSON parse tree depth level of 5. QUESTIONDid we answer your question with the above text? UPDATE 0Perhaps the high level question raised in GH-ssuecomment-2272414067 should be addressed first before trying to determine details about the current "jsemtblgen / patch" mechanism that, while it functioned for The need for JSON semantic checking the JSON parse tree for tools that link to Perhaps a better and different approach is needed altogether. |
Copied from GH-issuecomment-2273674353:
We think that the "## Problem" section of the GH-issuecomment-2272443630 is still a bit of a problem. Given the "## Suggestion" section of the GH-issuecomment-2272443630 had little effect, we speculate that the above line was the only sentence that was timely in need of answering. 😄 😄 😄 AnswerWe will write a new comment with a new suggestion based on the question we raised in GH-issuecomment-2272414067. When we complete our next comment we speculate that we will have provided better content, to move us forward, than if we had responded to all of the direct and implied questions from GH-issuecomment-2273496974. However for that next comment we are promising above, you will have to wait a bit while we ponder the answer to our question we raised in GH-issuecomment-2272414067. UPDATE 0We we are in a hurry to complete If we forget to do so in a few days, please remind us, @xexyl. 🤓 |
Copied from GH-issuecomment-2282227580:We have a concept for testing JSON semantics by use of JSON. We are thinking of how illustrate our idea with a simple tool in the near future, for certain values of near. |
Copied from GH-issuecomment-2293965292:In regards to GH-issuecomment-2282227580: The high level idea is to have add command line option to We are woking out the details of the format of those "JSON semantic strings", so expect a later comment describing them. The "JSON semantic string" output could be modified by a developer of a JSON semantic checking tool (such as In the generic case, semantic checking code (such as Then by "walking over" both JSON parse trees in memory in parallel, one could use the JSON semantic reference parse tree to examine the parse tree of JSON input the question. We say "walking over" because when one encounters a "JSON semantic string" indicating that the corresponding JSON value is optional or that the corresponding JSON value could be repeated (with a possible min and/or max repeat count), one as to "walk around" the JSON parse tree of the JSON in question (such as the JSON parse tree of The "JSON semantic string" will also contain an optional reference to a "chk-function" that, when called, will perform various value specific checks (similar to functions found in The result of the above architecture will be a more generic way that you and others can check the semantics of any form of JSON. FYI, More TBDThis is just an FYI for the curious and is not intended to be a complete technical specification. Please wait as more details relating to the above are developed and modified and deleted and replaced. :-) Please also wait as the specification of the format of the "JSON semantic string" is being developed. Such detailed info will be posted as comments below. Stay tuned over the next several days. And if you don't see more of the promised above content posted below soon enough (because we may have gotten so busy with work on matters related to GH-issuecomment-2293850933, please remind us after a few days. |
Copied from GH-issuecomment-2294938385:
The |
Copied from GH-issuecomment-2295015680:
The python submit server will need to be in advanced enough state before the Great Fork Merge in order that screen shots may be added to FAQ(s) related to how to submit to the IOCCC. The "register for the IOCCC" process will need to be in advanced enough state before the Great Fork Merge in order that screen shots may be added to FAQ(s) related to how to register for the IOCCC. Before IOCCC28 goes into "pending" state the BETA submit server will need to be up and running so that people may test using the Before IOCCC28 goes into "pending" state the BETA "register for the IOCCC" process will need to be and running so that people (including the IOCCC judges) may test the submit server workflow. The "screen sorts" before the * Great Fork Merge AND these BETA registration & BETA submit server take the place of what was once called IOCCCMOCK. |
Copied from GH-issuecomment-2310926005:
Performed on that other repo with commit 7a3907c. |
Copied from GH-issuecomment-2311005389:We have "cloned" this enhancement request into issue #940 in the mkiocccentry repo. |
Please assign. |
I just (and that's all I'm doing for now as we have to figure out the semantics - plus other more important things) made a slight improvement over the current design (or so I think). It is now possible to specify the author/ directory and the .entry.json filename to search/open. I know it's unlikely that it'll ever be run from a directory that's not the website root directory but now it should be possible to do so if one should choose to do so. I made an improvement to an error message and once that's merged I can pull and then add back the changes (from backup or else I am going to look at the CSS issue over there and then I have other things to do. Please do make a new release if you need to and again THANK YOU for holding off! I really appreciate it very much and I'm sorry for the delay. |
I have a suggestion. It's not necessary to do before the merge, maybe, but it would be good to do before the parser is moved to the jparse repo. The function What do you think? |
That seems like a good idea. |
When would you like me to do that? |
Yes, please. We added this as a TODO so then when you work on this issue, the function can be moved at that time. |
Well I might even end up doing it today because there's an unfortunate extraneous word in two error messages that I could imagine someone wanting to try and fix which would trample over my changes. Not at laptop yet but will be in a little while and likely will look at it then. I have to take a shower and do some other things. There will be interruptions here and there but I more than anything today want to tackle 2005. Hope you and I both have a better day today (I didn't have that great of a day yesterday either: actually it was a poor day)! |
Would such a change warrant the making of a new release of the repo? If the answer is no, then we advise against such a change as those picking up the released code won't see it. |
Fair enough. Then what can we do about people maybe wanting to fix it? If they did it might cause a conflict with my changes. But I will avoid making the changes for now. When we have a better idea about semantics I will probably start thinking about the updates. In a little bit I HOPE to work on 2005 but it probably will be a bit more time. I might take the opportunity to work on those other things that take less focus as I am still waking up. Then after that I can look at 2005. In the meantime .. |
The same thing if someone, while testing the code finds an execution flaw or portability issue that needs to be fixed. Such an emergency patch will need to be applied to any other code branch. UPDATE 0There's always a risk that code will need to be updated to fix a functional problem: especially a fix that is required for all contestants that forces the use of a new version number of a critical tool. Such a fix would force a new repo release and force everyone to recompile with the new code to produce the correct version in the JSON files. On the other hand, fixing a typo in a man page, or comment, or even an error message wording (unless that resulted in a significantly misleading set of instructions), while annoying, would not be a change that is necessary for everyone to adopt and recompile. |
Sure. |
Will do that shortly (for certain definitions of shortly) and then take care of some other things. If I have time after that I will try working with the other repo. |
Done with 03f605c. You can thus mark that todo item complete if you wish. Once the semantics details (and other things necessary) have been discussed more I can further enhance the tool, getting more insight into and familiarity with the dynamic array facility and some of the lower level json functions that you added. I have to take care of other things but if I have time after that I will work on 2005. The good news is that if I do not have time it seems that the next thing I have to work on will be 2005 (though I have a couple other html files fixed in format) .. unless of course yet another thing here requires updates! |
See the request to update the PR. |
We need to describe our model for how to make a "parallel" JSON parse tree that contains specially formatted JSON strings that describe how to evaluate the semantics of a JSON tree. It is a shame that we don't have a simple If we had such a tool, then it would be a straightforward process to add a command like option (such as Perhaps it is time for you to update your jparse repo with the current state of the If you did that, then we would populate our
Then you could add a simple
We could then fork your updated jparse repo with jprint added and issue to you a PR that would modify simple TL;DRWe recommend:
|
The above sounds great! I need to get some food soon and then if I have to I will do this (I think I am about done with the other stuff). I am tempted to do this first but I feel like it might take some time and work so I better eat first. If I don't get to it today I will do it tomorrow. Or if you want you could issue a pull request. Assuming that I have finished the other things and I don't have to leave for the day I might be able to work on this in say half an hour or so. Perhaps after this I can also work out how to integrate the parser into the test suite we added JSON files from. As for not adding code with functionality changes no worries. |
Of course the jprint tool might take a bit of time. But on that note how does this (in priority) (the jprint tool I mean) compare to the html files in the other repo? Thanks! |
It is a lower priority. Perhaps something that can be done when the final stages of the Great Fork Merge are underway (and post html files in the other repo are done). We will hold off on describing the |
But should the repo be populated now (I know you said it should be but since we are talking about one part waiting I want to be sure)? |
One moment please ... |
Sure. Food still cooking. |
Hope it is cooking well. See PR #1 in your jparse repo. The first pull request for that repo! 🤓 |
It's already in my alimentary canal, breaking down and so on, the poor little things. Thanks!
I was hoping it would be you actually! Unfortunately some build problems. But I merged so we can address it. One issue (not sure if it's the only issue) is that the json semantic table generator requires |
Thanks for accepting the PR #1. We then did for our own tree under this repo: make jparse.update_clone Now when we do: make jparse.diff_jparse_clone we only see: diff -u -r --exclude-from=.exclude jparse.clone jparse
Only in jparse.clone: CHANGES.md
make: [jparse.diff_jparse_clone] Error 1 (ignored) |
Well the problem here (I think) can be solved by removing the |
We should be discussing this in that other repo .. but for now we had to place our clone under
For now, we commend you try the above. Eventually in your GitHub workflow, you will need address this. Currently the jparse tree assumes the above. |
I added a comment there asking what you think since of course the workflow needs to have it but so does anyone using the parser. It already builds locally but nobody else can unless they have installed the other repos. |
Added a TODO about |
Thanks. Perhaps there should be a new issue about this in the jparse repo? Please feel free to open one. That way we can discuss it and there can be a clearer idea of what you are after. |
See issue #5 in the jparse repo. Please NOTE: The issue is just a draft. We will go to sleep, wake up and if needed, revise it. |
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. |
When issue #979 is completed, the |
Yes but this comes after the next IOCCC, right? Okay off again for the day. Back tomorrow. |
I just noticed something useful for this. The |
Is there an existing issue for this?
Describe the feature
Currently
chkentry(1)
validates the .info.json and .auth.json files.As of commit 7a3907c
chkentry(1)
throws a usage error of the form when given a single argument:We propose that when given a directory of the form
YYYY/dir
(whereYYYY
is an IOCCC year anddir
is the name of a winning IOCCC entry found underYYYY
) thatchkentry(1)
should validate the semantics of the entry's.entry.json
and relatedauthor/author_handle.json
files. This will also include verifying that the files referenced in the.entry.json
exist, etc.The faq.md of the other repo and the
FAQ.md
of this repo will need to be updated accordingly.Relevant images, screenshots or other files
Read at least 42 words of humor from the closed issue #2614 as found in the temp-test-ioccc repo for some fun.
Relevant links
From the closed issue #2614 as found in the temp-test-ioccc repo we recommend:
These comments have been duplicated below.
Anything else?
Stay tuned as we transfer important comments from the closed issue #2614 as found in temp-test-ioccc repo below.
Additional TODO
Move
open_json_dir_file()
inchkentry.c
intojparse/util.c
Write a
jprint(1)
tool and add it to the jparse repoSee GH-issuecomment-2316164972 for information on the
jprint(1)
tool and why it is needed for this issue.The text was updated successfully, but these errors were encountered: