search

xach / buildapp

Buildapp makes it easy to build application executables with SBCL
http://www.xach.com/lisp/buildapp/
123 stars 27 forks source link

Replace "dynamic-space-size" argument by "space-size". #39

Open rpgoldman opened 10 months ago

rpgoldman commented 10 months ago

This change was required to fix the problem of the SBCL executable that is running buildapp gobbling up the dynamic-space-size command line argument instead of letting it be a setting for the image that is being created.

Fixes #36

rpgoldman commented 10 months ago

@xach If you think this is OK, I will update the documentation to agree with it.

On the other hand, if you think it's a bad approach, maybe you could tell me how better to handle --dynamic-space-size

xach commented 10 months ago

This seems like a bug in SBCL to me. The image should get all command line options unmodified. Do you know when this changed in SBCL?

On Thu, Jan 11, 2024 at 2:25 PM rpgoldman @.***> wrote:

This change was required to fix the problem of the SBCL executable that is running buildapp gobbling up the dynamic-space-size command line argument instead of letting it be a setting for the image that is being created.

Fixes #36

You can view, comment on, or merge this pull request online at:

https://github.com/xach/buildapp/pull/39

Commit Summary

eb2ac88 Replace "dynamic-space-size" argument by "space-size".

File Changes (1 file)

M buildapp.lisp (81)

Patch Links:

https://github.com/xach/buildapp/pull/39.patch https://github.com/xach/buildapp/pull/39.diff

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.Message ID: @.***>

rpgoldman commented 10 months ago

I'm afraid I don't. I only last year started making applications for which I wanted to set the dynamic-space-size, so this has always failed for me.

Is this something that could be fixed by inserting --end-runtime-options into the argument stream?

Looks like I also added the LDB-related option. I must have done this so long ago that I forgot doing it!

Actually, it seems like disabling LDB should be the default, and the user should need to set an option to turn it on: for most cases it won't be helpful to drop into the LDB and will be better to simply exit.

xach commented 10 months ago

If buildapp isn't getting --dynamic-space-size passed through, there's either a bug in how it's saving the executable with save-lisp-and-die, or a bug in SBCL. The :save-runtime-options documentation says this:

:save-runtime-options

If true, values of runtime options –dynamic-space-size and –control-stack-size that were used to start sbcl are stored in the standalone executable, and restored when the executable is run. This also inhibits normal runtime option processing, causing all command line arguments to be passed to the toplevel. Meaningless if :executable is nil.

I implemented this 15 years ago, specifically for buildapp command-line processing.

rpgoldman commented 10 months ago

If buildapp isn't getting --dynamic-space-size passed through, there's either a bug in how it's saving the executable with save-lisp-and-die, or a bug in SBCL.

If that argument is being passed through, it should appear in the dumper, correct? If so, I should be able to provide a test for this and add it to my MR, and you can check that and either accept the MR or see about the possibility of an SBCL bug. I'll get to this as soon as I can manage.

xach commented 10 months ago

I would expect it to appear in the dumper, yes.

I won't accept an MR that changes the command-line argument name, though. And all the extra debug output is not acceptable either.

On Tue, Jan 23, 2024 at 9:38 AM rpgoldman @.***> wrote:

If buildapp isn't getting --dynamic-space-size passed through, there's either a bug in how it's saving the executable with save-lisp-and-die, or a bug in SBCL.

If that argument is being passed through, it should appear in the dumper, correct? If so, I should be able to provide a test for this and add it to my MR, and you can check that and either accept the MR or see about the possibility of an SBCL bug. I'll get to this as soon as I can manage.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

rpgoldman commented 10 months ago

I'm not sure how I can demonstrate this issue without adding some debugging prints. The dumpfile has save-runtime-options, but the issue I am seeing is that the argument does not get into the dumper object. It has a slot for dynamic-space-size which does not seem to get set. The command-line file tries to parse a dynamic-space-size argument, so I believe that slot of the dumper should be set, correct? In my checks, it was not being set because SBCL did not let buildapp see that argument.

I am putting my test in a merge request that should not be merged -- instead you can check out the branch and run the test to see the problem.