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.

Sign up for GitHub

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

Jump to bottom

Allow safe stripping of Bazel binary #11842

Open
olekw opened this issue Jul 27, 2020 · 8 comments
Open

Allow safe stripping of Bazel binary #11842

olekw opened this issue Jul 27, 2020 · 8 comments
Assignees
@meteorcloudy
Labels
not stale Issues or PRs that are inactive but not considered stale P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Rules-CPP Issues for C++ rules type: documentation (cleanup)

Comments

@olekw
Copy link
Contributor

olekw commented Jul 27, 2020

Description of the problem / feature request:

Please consider implementing a means of safely stripping the Bazel binary. I see two possible means by which this might be achieved:

  1. Document strip command options that would allow debug symbols to be removed without also removing the embedded zip file.

  2. Provide (or document, if it exists already) a means by which a "skinny" Bazel binary could be built that would not contain the embedded zip and could therefore be safely stripped. This is the preferred solution from a Debian perspective since we are already packaging Bazel with an extracted system-level installation base that we point to upon execution with --install_base.

Feature requests: what underlying problem are you trying to solve with this feature?

Stripping release binaries is common practice and even required in many cases for both speed and storage efficiency. For example, this is a requirement in Debian and produces a packaging error if skipped.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Strip the Bazel binary with strip --strip-unneeded or strip --strip-debug.

$ bazel version
Opening zip "/proc/self/exe": open(): No such file or directory
FATAL: Failed to open '/proc/self/exe' as a zip file: (error: 2): No such file or directory

What operating system are you running Bazel on?

Debian Unstable

What's the output of bazel info release?

Same as bazel version above.

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

Built as part of Debian package for #9408.

Have you found anything relevant by searching the web?

Yes. The stripping problem seem to be a known issue as shown in #600.
@olekw
Copy link
Contributor Author

olekw commented Jul 27, 2020

Following up on Solution 2 above: a simple way of achieving that might be if the C++ launcher skipped looked for the embedded zip when the --install_base option is present. That would allow the binary to be stripped, and the embedded zip removed, without impacting functionality or requiring any extensive changes in Bazel.

@olekw
Copy link
Contributor Author

olekw commented Jul 28, 2020

So perhaps a check like this could be extended to the other locations (I believe all in the blaze.cc file) where Bazel tries to read from the embedded archive?

@oquenchil oquenchil added team-Rules-CPP Issues for C++ rules type: documentation (cleanup) P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) labels Jul 28, 2020
@olekw
Copy link
Contributor Author

olekw commented Aug 6, 2020

@meteorcloudy This is the big holdup right now. I've tried a couple patches to implement what I suggest here but I can't get them to work...

@meteorcloudy meteorcloudy self-assigned this Aug 6, 2020
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Aug 7, 2020
@meteorcloudy
Add --trust_install_base option
ea05a8d 
This will allow Bazel client to run individually without the appended
zip archive, which is a requirement for Debian installation.

The following changes are made:
  - Add --trust_install_base option. When this is enabled. Bazel client
    trusts the install base provided by --install_base option and skip
    the step of verifying the install directory. We need it because the
    Bazel client alone doesn't contain information to verify the install
    directory.
  - Reimplement --version as a actual Bazel startup flag. The version
    info is read from the zip archive, but when --trust_install_base is
    enabled it should be read from the given install base.

Related: bazelbuild#9408
Fixes: bazelbuild#11842
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Aug 7, 2020
@meteorcloudy
Add --trust_install_base option
2855115 
This will allow Bazel client to run individually without the appended
zip archive, which is a requirement for Debian installation.

The following changes are made:
  - Add --trust_install_base option. When this is enabled. Bazel client
    trusts the install base provided by --install_base option and skip
    the step of verifying the install directory. We need it because the
    Bazel client alone doesn't contain information to verify the install
    directory.
  - Reimplement --version as a actual Bazel startup flag. The version
    info is read from the zip archive, but when --trust_install_base is
    enabled it should be read from the given install base.

Related: bazelbuild#9408
Fixes: bazelbuild#11842
@meteorcloudy meteorcloudy mentioned this issue Aug 7, 2020
Closed
@meteorcloudy meteorcloudy added P2 We'll consider working on this in future. (Assignee optional) and removed P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) labels Aug 7, 2020
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Aug 10, 2020
@meteorcloudy
Add --trust_install_base option
2abd836 
This will allow Bazel client to run individually without the appended
zip archive, which is a requirement for Debian installation.

The following changes are made:
  - Add --trust_install_base option. When this is enabled. Bazel client
    trusts the install base provided by --install_base option and skip
    the step of verifying the install directory. We need it because the
    Bazel client alone doesn't contain information to verify the install
    directory.
  - Reimplement --version as a actual Bazel startup flag. The version
    info is read from the zip archive, but when --trust_install_base is
    enabled it should be read from the given install base.

Related: bazelbuild#9408
Fixes: bazelbuild#11842
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Aug 10, 2020
@meteorcloudy
Add --trust_install_base option
798831b 
This will allow Bazel client to run individually without the appended
zip archive, which is a requirement for Debian installation.

The following changes are made:
  - Add --trust_install_base option. When this is enabled. Bazel client
    trusts the install base provided by --install_base option and skip
    the step of verifying the install directory. We need it because the
    Bazel client alone doesn't contain information to verify the install
    directory.
  - Reimplement --version as a actual Bazel startup flag. The version
    info is read from the zip archive, but when --trust_install_base is
    enabled it should be read from the given install base.

Related: bazelbuild#9408
Fixes: bazelbuild#11842
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Aug 20, 2020
@meteorcloudy
Add --trust_install_base option
96598e2 
This will allow Bazel client to run individually without the appended
zip archive, which is a requirement for Debian installation.

The following changes are made:
  - Add --trust_install_base option. When this is enabled. Bazel client
    trusts the install base provided by --install_base option and skip
    the step of verifying the install directory. We need it because the
    Bazel client alone doesn't contain information to verify the install
    directory.
  - Reimplement --version as a actual Bazel startup flag. The version
    info is read from the zip archive, but when --trust_install_base is
    enabled it should be read from the given install base.

Related: bazelbuild#9408
Fixes: bazelbuild#11842
meteorcloudy added a commit to meteorcloudy/bazel that referenced this issue Aug 20, 2020
@meteorcloudy
Add --trust_install_base option
ca6efc1 
This will allow Bazel client to run individually without the appended
zip archive, which is a requirement for Debian installation.

The following changes are made:
  - Add --trust_install_base option. When this is enabled. Bazel client
    trusts the install base provided by --install_base option and skip
    the step of verifying the install directory. We need it because the
    Bazel client alone doesn't contain information to verify the install
    directory.
  - Reimplement --version as a actual Bazel startup flag. The version
    info is read from the zip archive, but when --trust_install_base is
    enabled it should be read from the given install base.

Related: bazelbuild#9408
Fixes: bazelbuild#11842
@mr-c
Copy link

mr-c commented Nov 19, 2020

Hello @oquenchil , why was this closed? I don't think #11905 was merged yet?

@oquenchil
Copy link
Contributor

Re-opening and marking as P4. @meteorcloudy if you ever restart that work please mark as P2.

@oquenchil oquenchil reopened this Nov 19, 2020
@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed P2 We'll consider working on this in future. (Assignee optional) labels Nov 19, 2020
@isaackwy isaackwy mentioned this issue Aug 27, 2022
Merged
@Apteryks
Copy link

Apteryks commented Oct 17, 2022 •
edited
Loading

I think this can be closed, given #11905 is now merged. Nevermind, it was not merged but closed without resolution.

@keertk keertk added the team-Documentation Documentation improvements that cannot be directly linked to other team labels label Jan 10, 2023
@isaackwy isaackwy mentioned this issue Mar 31, 2023
Closed
@github-actions GitHub Actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Mar 16, 2024
@mr-c
Copy link

mr-c commented Mar 16, 2024

This is still needed

@brentleyjones brentleyjones removed the stale Issues or PRs that are stale (no activity for 30 days) label Mar 16, 2024
@brentleyjones brentleyjones added the not stale Issues or PRs that are inactive but not considered stale label Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@meteorcloudy meteorcloudy

Labels
not stale Issues or PRs that are inactive but not considered stale P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Rules-CPP Issues for C++ rules type: documentation (cleanup)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add --trust_install_base option meteorcloudy/bazel
7 participants
@brentleyjones @mr-c @olekw @meteorcloudy @Apteryks @oquenchil @keertk