Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Sep 20, 2024

The subprocess creation on windows really doesn't like it when an
argument contains < or >. Instead use the default
FT_CONFIG_CONFT_CONFIG_CONFIG_H and define the two
HAVE_ macros on the command line.

See #22585 (comment)

@sbc100 sbc100 changed the title [test] Mark freetype test as crossplaform. NFC [test] Mark freetype test as crossplaftorm. NFC Sep 20, 2024
@sbc100 sbc100 force-pushed the freetype_crossplatform branch 7 times, most recently from 9bf7d79 to 83d2848 Compare September 20, 2024 20:54
@sbc100 sbc100 changed the title [test] Mark freetype test as crossplaftorm. NFC Fix freetype build on windows and mark test as crossplaform Sep 20, 2024
@sbc100 sbc100 requested a review from dschuff September 20, 2024 20:55
- run-tests:
title: "crossplatform tests"
test_targets: "--crossplatform-only"
test_targets: "other.test_freetype_emscripten"
Copy link
Member

Choose a reason for hiding this comment

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

was this just for testing?

Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

The subprocess creation on windows really doesn't like it when an
argument contains < or >.  Instead of trying to defined
`FT_CONFIG_CONFIG_H` or point to our own version, just overwrite the
default version.

See emscripten-core#22585 (comment)
@sbc100 sbc100 force-pushed the freetype_crossplatform branch from 83d2848 to 5d7c55e Compare September 20, 2024 21:02
Copy link
Member

@dschuff dschuff left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

@dschuff dschuff merged commit ff46872 into emscripten-core:main Sep 23, 2024
11 of 13 checks passed
@sbc100 sbc100 deleted the freetype_crossplatform branch September 23, 2024 20:25
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.

3 participants