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

Use unstripped executables when running tests on Linux #26921

Merged

Conversation

jason-simmons
Copy link
Member

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

This patch looks good but WDYT of the suggestion?

# stack traces on Linux.
executable = os.path.join(build_dir, "exe.unstripped", executable_name)
# Some tests depend on the EGL/GLES libraries placed in the build directory.
env = {"LD_LIBRARY_PATH": build_dir}
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should just modify the build output directory for unstripped executable to just be next to stripped variants. That way, we won't have to do library path manipulations or extra flags. Perhaps just have a suffix to the executable name (exe_name_unstripped). This will also help in the cases where folks need to attach debuggers and such to the unstripped exe runs.

I suppose the Right™ way to do this would be have stripping be a target specific option but right now its all or nothing unfortunately.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer to keep the current directory structure in order to maintain similarity to the original Chromium build rules (and other build rules like Dart's that might assume Chromium's setup)

Most of the engine tests can run directly out of exe.unstripped (in particular, the auto-generated GetFixturesPath paths are fully qualified and do not depend on the location of the executable). But the embedder tests that rely on SwiftShader EGL/GLES must have a way to locate those libraries.

Copy link
Member

Choose a reason for hiding this comment

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

Sure. It's your call. LGTM either way.

@jason-simmons jason-simmons force-pushed the absl_run_tests_unstripped branch 3 times, most recently from 48ed316 to 9417c39 Compare June 28, 2021 23:21
@jason-simmons jason-simmons added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jun 29, 2021
@fluttergithubbot fluttergithubbot merged commit 56884b7 into flutter:master Jun 29, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 29, 2021
moffatman pushed a commit to moffatman/engine that referenced this pull request Aug 5, 2021
naudzghebre pushed a commit to naudzghebre/engine that referenced this pull request Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
3 participants