-
Notifications
You must be signed in to change notification settings - Fork 114
Rewrite packbeam.c #1429
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
base: main
Are you sure you want to change the base?
Rewrite packbeam.c #1429
Conversation
I did expect some CI to fail but not pretty much everything. How can I test it locally? I was doing In any case, I'll try to pin down the issue. You don't need to do full review, just let me know if approach is sane. |
See draft PR #1510 as a partial solution to your CI problems... But after pulling your branch and cherry-picking the changes for #1510 it appears that your implementation is not recognizing that when For example when I try the build the esp32boot target I get an internal error from PackBeam:
PackBeam should be creating the file |
I don't think #1510 should be necessary, but the help in the current |
Also, one feature missing from the C implementation that the pure erlang module has is a |
free(file_data.data); | ||
return ret; | ||
} | ||
TRY(safe_read_file(output_file, &file_data)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a problem. Trying to read and verify that the not yet created avm archive is valid... this check should be done after the files have been packed into the output archive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can successfully create an archive with lines 209-214 commented out.
I went and implemented my suggestion from #1394.
I didn't do any thorough checks yet since I want to get feedback first (
make -j
inbuild/
doesn't crash on my machine).Worked on original file, slowly refactoring to what I had in mind. Do we have tests for packbeam or we only check it by usage?
Leaving all commits for now, will squash after acceptance.
TRY
macro that checks bool output from expression and iffalse
, stops the execution of current function.bool
output to check for errors.perror
for internals?).These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).
SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later