Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 7, 2019

We already use a common "lib" directory to install al the libs. This
change simply installs headers in the same common "sysroot".

This makes the ports install more like normall "make install" scripts
would.

The only port I didn't include here was cocos2d because it seemed a
little overwhelming.

@sbc100 sbc100 requested a review from dschuff December 7, 2019 03:55
@sbc100 sbc100 force-pushed the common_include_dir branch from 114ba7d to 2afe56a Compare December 7, 2019 18:00
@sbc100 sbc100 requested a review from kripken December 7, 2019 21:28
Copy link
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm, although I am slightly worried we may not have enough test coverage to catch corner cases in all the ports...

embuilder.py Outdated
help='force rebuild of target (by removing it first)')
parser.add_argument('operation')
parser.add_argument('targets', nargs='+')
parser.add_argument('operation', help='currnetly only "build" is supported')
Copy link
Member

Choose a reason for hiding this comment

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

typo in currently

includes = [i.strip() for i in includes.splitlines()[1:-1]]
for i in includes:
self.assertContained(path_from_root('system'), i)
if shared.Cache.dirname in i:
Copy link
Member

Choose a reason for hiding this comment

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

this isn't clear to me. perhaps a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry this isn't best test in the world. Adding a comment.

We already use a common "lib" directory to install al the libs.  This
change simply installs headers in the same common "sysroot".

This makes the ports install more like normall "make install" scripts
would.

The only port I didn't include here was cocos2d because it seemed a
little overwhelming.
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 9, 2019

Added a ChangeLog entry since this change could be user visible in some cases.

@sbc100 sbc100 force-pushed the common_include_dir branch from 94067bc to b8d95e3 Compare December 9, 2019 22:07
@sbc100 sbc100 merged commit 5d222c1 into incoming Dec 9, 2019
@delete-merged-branch delete-merged-branch bot deleted the common_include_dir branch December 9, 2019 23:59
sbc100 added a commit that referenced this pull request Dec 12, 2019
sbc100 added a commit that referenced this pull request Dec 12, 2019
Also add basic sdl2_mixer test outside of browser tests that just
tests for successful compilation.

Fixes #10015
sbc100 added a commit that referenced this pull request Dec 13, 2019
Also add basic sdl2_mixer test outside of browser tests that just
tests for successful compilation.

Fixes #10015
sbc100 added a commit that referenced this pull request Dec 16, 2019
Also add basic sdl2_mixer test outside of browser tests that just
tests for successful compilation.

Fixes #10015
Beuc added a commit to Beuc/emscripten that referenced this pull request Mar 12, 2020
This isn't necessary anymore since emscripten-core#9983 / 5d222c1 (common include/ directory).
kripken pushed a commit that referenced this pull request Mar 17, 2020
This isn't necessary anymore since #9983 / 5d222c1
(common include/ directory).
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
We already use a common "lib" directory to install all the libs.  This
change simply installs headers in the same common "sysroot".

This makes the ports install more like normal "make install" scripts
would.
belraquib pushed a commit to belraquib/emscripten that referenced this pull request Dec 23, 2020
…ten-core#10021)

Also add basic sdl2_mixer test outside of browser tests that just
tests for successful compilation.

Fixes emscripten-core#10015
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