ioccc-src / mkiocccentry

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

[Bug] building only iocccsize does not generate limit_ioccc.sh #884

Closed SirWumpus closed 4 months ago

SirWumpus commented 4 months ago

Is there an existing issue for this?

Describe the bug

Building only iocccsize does not generate limit_ioccc.sh:

elf$ gmake iocccsize
...
elf$ ./test_ioccc/iocccsize_test.sh
./test_ioccc/iocccsize_test.sh: ERROR: limit_ioccc.h does not exist: ./soup/limit_ioccc.sh
elf$

Its assumed that all the tools are generated before running any tests.

What you expect

I should be able to build an individual tool and run their respective test; any required files should have been generated already.

Environment

- OS: NetBSD 10.0
- Device: Guest VM on Windows 10 Home with VirtualBox
- Compiler: gcc version 10.5.0 (nb3 20231008)

bug_report.sh output

No response

Anything else?

Did I forget to turn off the stove?

xexyl commented 4 months ago

Actually I at one point suggested that it might compile all first but this was thought the wrong approach.

You might also not realise that some of the tools use other tools so trying to run specific tests can be problematic without compiling all.

If however all are compiled you actually can run individual tests by running those individual tests.

It seems not important to me (but of course you would have to ask Landon 😀) to be able to run tests without compiling all code since numerous tools use others.

As for the generated file there MIGHT be a rule to generate it. Still you can't run several of the tests without compiling all the code.

xexyl commented 4 months ago

Very funny additional comments btw: very much appreciated!

xexyl commented 4 months ago

This I will take care of as soon as I have the laptop back up. Doing the computer room and my bedroom first so it shouldn't be too much longer I think.

After this I will go back to the task I have to do before working on the other repo again.

This one shouldn't take long at all and if there's no Makefile rule I will add one. Back soon as I can!

xexyl commented 4 months ago

Okay the file is generated it's just that the test script cannot find it ... Looking at it more.

xexyl commented 4 months ago

Okay the file is generated it's just that the test script cannot find it ... Looking at it more.

Oh .. right. That's the script it's after. Okay this is indeed odd. Well now that I have the file name right it should be easier to fix.

xexyl commented 4 months ago

I have identified the problem if problem is the right way of putting it.

The thing is it's under soup/. Do we want to have the make rules always build soup? For instance the following diff would fix it but only for iocccsize:

diff --git i/Makefile w/Makefile
index 8f2f042cf264ba434e7ff75f730d61ce83c71e62..9e985258defa0e695177e3c640612777bbafe8fc 100644
--- i/Makefile
+++ w/Makefile
@@ -434,10 +434,11 @@ mkiocccentry: mkiocccentry.o soup/soup.a jparse/jparse.a dyn_array/dyn_array.a d

 iocccsize.o: iocccsize.c
        ${CC} ${CFLAGS} -DMKIOCCCENTRY_USE iocccsize.c -c

 iocccsize: iocccsize.o soup/soup.a dbg/dbg.a
+ ${Q} ${MAKE} ${MAKE_CD_Q} -C soup limit_ioccc.sh
        ${CC} ${CFLAGS} $^ -o $@

 txzchk.o: txzchk.c
        ${CC} ${CFLAGS} txzchk.c -c

but it will also build a bunch of other things which might not be desirable: the parser (including verge), the dyn_array and dbg libaries and other things.

And what's more is that it would require the command in every rule.

Is this really wanted? Maybe it's only needed for some? But even so if doing this it means that other things will be built and that includes things that are not needed for just building iocccsize.

Please advise.

xexyl commented 4 months ago

What's strange is now I see that building iocccsize already does build those things. Maybe I forgot to run make clobber earlier times? I was pretty sure I had though. So the question is why is it not generating the shell script file when it does build soup.a?

Ah! I found the problem I think. If so I'll fix it and make a commit.

xexyl commented 4 months ago

What's strange is now I see that building iocccsize already does build those things. Maybe I forgot to run make clobber earlier times? I was pretty sure I had though. So the question is why is it not generating the shell script file when it does build soup.a?

Ah! I found the problem I think. If so I'll fix it and make a commit.

UPDATE 0

Well actually it's not the fix but it is something else. The reason that it was building those other things now is that I had updated the Makefile to test. And the reason that running make from soup/ will generate the sh file is done in a way that seems odd. I'll make a change and commit even though this file will still not be built unless one does something like what I listed above (or I guess a better way would be to make it a dependency of the rule).

UPDATE 1

Looking at it again it seems maybe it's not a good idea, the change I had just saw as a possibility. I was guessing that the SH_TARGETS might be in TARGETS but they're not. I'm not sure if they should be or not. If they should be then the limit_ioccc.sh could be moved to SH_TARGETS and the TARGETS could have SH_TARGETS instead. But I haven't looked beyond that and for now I must leave.

UPDATE 2

As I suspected it's not a good idea. So unless we want to run make -C soup limit_ioccc.sh for specific rules or we want to move it to / it doesn't seem like it's going to work - unless we change the way things work.

xexyl commented 4 months ago

I have a fix. Will commit soon ..

xexyl commented 4 months ago

See 94db6f474911492ecefffaae4e4ea944d3d9531c.

But as I said in a comment about how it's done:


Again feel free to clean up or suggest another way to go about it. It does make me wonder that since it previously would only be built if one built everything that maybe it could be devised better. How that might be though I don't have an idea at this time.

I don't know that I like it at the soup.a library but since these tools rely on soup.a and thus will be building them and since it's in soup it seems like a place it can be.


If you wish to clean it up or if you have something else we should do in mind please let me know. But since it is part of the soup and since the tools have to build soup (and thus run that rule) it works. Perhaps other rules could be updated instead?

lcn2 commented 4 months ago

Do the recent changes fix this issue for you, @SirWumpus ?

SirWumpus commented 4 months ago

Hmm. Error is gone, but there is no output.

elf$ gmake clobber iocccsize
...
elf$ ./test_ioccc/iocccsize_test.sh
elf$ echo $?
0
xexyl commented 4 months ago

Hmm. Error is gone, but there is no output.


elf$ gmake clobber iocccsize

...

elf$ ./test_ioccc/iocccsize_test.sh

elf$ echo $?

0

Try -v 1.

SirWumpus commented 4 months ago

Better...

./test_ioccc/iocccsize_test.sh: all tests PASSED
lcn2 commented 4 months ago

Thank you 🙏 @SirWumpus for bringing this issue to our attention and for testing the fix.

SirWumpus commented 4 months ago

It wasn't a huge issue, I could have lived with being told "build it all", but its a nice "to have" now that its done.

Poking holes and asking questions now is better than later during a running contest, n'est pas?

xexyl commented 4 months ago

It wasn't a huge issue, I could have lived with being told "build it all", but its a nice "to have" now that its done.

I understand that. It feels more correct now. It might seem silly but it is a useful file to be generated.

Poking holes and asking questions now is better than later during a running contest, n'est pas?

Indeed.