Skip to content

Fix poppler benchmark environment flags - #27544

Open
brendandahl wants to merge 1 commit into
emscripten-core:mainfrom
brendandahl:fix-poppler
Open

Fix poppler benchmark environment flags#27544
brendandahl wants to merge 1 commit into
emscripten-core:mainfrom
brendandahl:fix-poppler

Conversation

@brendandahl

Copy link
Copy Markdown
Collaborator

Preserve existing environment flags in the test library builder instead of overwriting them. This ensures caller-provided optimization settings are maintained.

Set CXXFLAGS alongside CFLAGS to ensure C++ dependencies compile with the correct settings. Pass environment overrides to the freetype dependency to fix the poppler benchmark build with LTO.

Preserve existing environment flags in the test library builder instead
of overwriting them. This ensures caller-provided optimization settings
are maintained.

Set CXXFLAGS alongside CFLAGS to ensure C++ dependencies compile with
the correct settings. Pass environment overrides to the freetype
dependency to fix the poppler benchmark build.
Comment thread test/common.py
env_init.setdefault('CFLAGS', cflags)
env_init.setdefault('CXXFLAGS', cflags)
# Append library-specific cflags without overwriting caller-provided optimizations
env_init['CFLAGS'] = f"{env_init.get('CFLAGS', '')} {cflags}".strip()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about env_init.setdefault('CFLAGS', '') += cflags ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do += with a function call.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.extend() then?

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.

2 participants