Skip to content
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

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

rpgoldman
Copy link

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

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.
@rpgoldman
Copy link
Author

@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

The latter is needed to fix the issue of SBCL swallowing the
`--dynamic-space-size` argument.

The former is necessary because the version in the GitHub repository was
missing the `--disable-ldb` initialization and handling code causing
failure to compile.
@xach
Copy link
Owner

xach commented Jan 16, 2024 via email

@rpgoldman
Copy link
Author

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.

Maybe the argument to `macroexpand-1` needed a quote?
SB-DEBUG:LIST-BACKTRACE is deprecated.
@xach
Copy link
Owner

xach commented Jan 23, 2024 via email

@rpgoldman
Copy link
Author

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
Copy link
Owner

xach commented Jan 23, 2024 via email

@rpgoldman
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

--dynamic-space-size handling on SBCL is broken
2 participants