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.

Sign up for GitHub

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

Jump to bottom

Added "make debug" which builds with DEBUG_F flag #1108

Merged
merged 5 commits into from
Jan 12, 2021
Merged

Added "make debug" which builds with DEBUG_F flag #1108

merged 5 commits into from
Jan 12, 2021

Conversation

hoodmane
Copy link
Member

DEBUG_F provides extra logging info: whenever FAIL is invoked, a messages is printed with a javascript stack trace, and the source line function and file where the failure happened. When #1080 is merged, it will add debug statements whenever an error occurs in EM_JS.

I also added a missing semicolon in the DEBUG_F version of FAIL.

@dalcde
Copy link
Contributor

dalcde commented Jan 11, 2021

Should debug enable -s ASSERTIONS=2 as well?

@hoodmane
Copy link
Member Author

Let me check how much difference that makes to the linking time.

@hoodmane
Copy link
Member Author

hoodmane commented Jan 11, 2021 •
edited
Loading

Okay they take the same amount of time so I'll add it, good idea.

@hoodmane
Copy link
Member Author

hoodmane commented Jan 11, 2021 •
edited
Loading

Btw, with -s ASSERTIONS=2 there's a bunch of warnings like:
warning: unexpected number of arguments 0 in call to 'hiwire_array', should be 1

Also, no matter what it always says:
emcc:INFO: Enabling --no-heap-copy because -s ALLOW_MEMORY_GROWTH=1 is being used with file_packager.py (pass --no-heap-copy to suppress this notification)

@hoodmane
Copy link
Member Author

hoodmane commented Jan 11, 2021 •
edited
Loading

Also instead of PYODIDE_PACKAGES="micropip" I should probably make it do all the packages used by the core tests.

@hoodmane
Copy link
Member Author

Oh this now has the problem that it's annoying to get an extra package built because the recipe writes over PYODIDE_PACKAGES. Maybe PYODIDE_PACKAGES+=...?

@hoodmane
Copy link
Member Author

@dalcde @rth I think this is a simple enough addition, would be helpful to merge (I will switch from using make minimal to make debug as my default).

dalcde
dalcde reviewed Jan 12, 2021
View reviewed changes
Makefile Outdated Show resolved Hide resolved
@dalcde dalcde merged commit 53e0a79 into pyodide:master Jan 12, 2021
@hoodmane hoodmane deleted the debug-make-recipe branch January 12, 2021 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dalcde dalcde dalcde left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hoodmane @dalcde