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.

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

Fix cross environment pollution. #1807

Merged
merged 1 commit into from
May 21, 2017

Conversation

oleavr
Copy link
Contributor

@oleavr oleavr commented May 16, 2017

Environment variables like CFLAGS and LDFLAGS should not affect the
cross environment.

Fixes #1772

@oleavr
Copy link
Contributor Author

oleavr commented May 16, 2017

Note: I realized my commit message style of the previous two PRs were
slightly off from the convention followed by @jpakkane. I've adjusted it
for these two PRs. Please let me know if I got it wrong. 😓

@jpakkane
Copy link
Member

This needs a unit test to ensure it won't get broken again. This requires creating a new unit test class, say LinuxArmCrossCompileTests and add a test that runs the setup with envvars set and using the arm cross file in the cross dir. Then check that the envvars are not in the build invocations. Thanks.

@oleavr oleavr force-pushed the cross-environment-pollution branch 2 times, most recently from d76d2b9 to 1b311f0 Compare May 21, 2017 18:16
@oleavr
Copy link
Contributor Author

oleavr commented May 21, 2017

@jpakkane Done.

@oleavr oleavr force-pushed the cross-environment-pollution branch from 1b311f0 to 3b6ed52 Compare May 21, 2017 18:22
run_tests.py Outdated
@@ -153,7 +153,7 @@ def is_cross_build(self):
print('Running unittests.\n')
units = ['InternalTests', 'AllPlatformTests']
if mesonlib.is_linux():
units += ['LinuxlikeTests']
units += ['LinuxlikeTests', 'LinuxArmCrossCompileTests']
Copy link
Member

Choose a reason for hiding this comment

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

These cross tests should only be done if the cross toolchain is installed. In run_tests it is done like this (at the end of the file):

shutil.which('arm-linux-gnueabihf-gcc-6') and not platform.machine().startswith('arm')

This should be extracted to a function should_run_linux_cross_tests() and used from both locations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpakkane Ahh nice catch! Thanks, will fix.

Environment variables like CFLAGS and LDFLAGS should not affect the
cross environment.

Fixes mesonbuild#1772
@oleavr oleavr force-pushed the cross-environment-pollution branch from 3b6ed52 to 383e561 Compare May 21, 2017 18:35
@oleavr
Copy link
Contributor Author

oleavr commented May 21, 2017

@jpakkane Fixed (hopefully).

@jpakkane jpakkane merged commit b595cda into mesonbuild:master May 21, 2017
@oleavr oleavr deleted the cross-environment-pollution branch May 21, 2017 23:06
InuSasha added a commit to InuSasha/LibreELEC.tv that referenced this pull request Mar 21, 2018
- generate cross-file per package, needed since mesonbuild/meson#1807 to use changed build-flags
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.

None yet

2 participants