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

Make cc_toolchain to always link (static|dynamic)_runtime_lib attributes #16520

Closed
yuzhy8701 opened this issue Oct 20, 2022 · 5 comments
Closed
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@yuzhy8701
Copy link
Contributor

Description of the feature request:

Currently, the (static|dynamic)_runtime_lib attributes of cc_toolchain are ignored when the feature static_link_cpp_runtimes is disabled. When static_link_cpp_runtimes is enabled, the toolchain will link static_runtime_lib statically in static mode, or dynamic_runtime_lib dynamically in dynamic mode.

This FR asks that when static_link_cpp_runtimes is disabled, cc_toolchain will link both the static_runtime_lib and dynamic_runtime_lib.

Ideally and ultimately, it should be something like cc_import:

  • cc_toolchain provides two runtime lib attributes, one for static mode and one for dynamic mode.
  • Both of them can take both static and dynamic libraries (like the srcs attribute of cc_library), and properly link by looking at file extensions.
  • If both are specified, bazel uses the static mode one for static linking mode, and dynamic one for dynamic linking mode.
  • If only one is specified, bazel uses that one in both linking modes.

What underlying problem are you trying to solve with this feature?

Context: #16309 (comment)

Currently, it is impossible to enforce at toolchain level, that a library is always dynamically (or statically) linked and in RPATH, regardless of link mode:

  1. If static_link_cpp_runtimes is disabled, no library can be injected at toolchain level (unless writing custom features, but dynamic libs will not be in RPATH).
  2. If static_link_cpp_runtimes is enabled, you can only inject static libraries in static linking mode (the same for dynamic mode).

But 1) some libraries can only be static / dynamic linked (e.g. sanitizer runtimes), or 2) a developer may choose to always dynamic link / static link libc++ / libstdc++. These use cases are not covered at the moment.

Which operating system are you running Bazel on?

Linux, Mac, Windows

What is the output of bazel info release?

release 6.0.0-pre.20220909.2

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

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

#16309

Any other information, logs, or outputs that you want to share?

No response

@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Oct 26, 2022
@oquenchil
Copy link
Contributor

Problem with this as far as I can see is that if we change the behavior of that existing feature we might break existing projects. But I agree that:

Currently, it is impossible to enforce at toolchain level, that a library is always dynamically (or statically) linked and in RPATH, regardless of link mode:

is a valid use case and the toolchain should allow this.

@brooksmoses, do you have any ideas related this?

@yuzhy8701
Copy link
Contributor Author

If changing existing feature is undesired, what about adding new fields? Maybe cc_toolchain can accept two new fields, or maybe create a new rule (cc_toolchain_import) that behaves like cc_import, with a link_when field specifying if this is for dynamic mode, static mode, or both modes...

@yuzhy8701
Copy link
Contributor Author

yuzhy8701 commented Jan 30, 2023

Actually, I feel like the cc rules can be refactored for better flexibility and clarity. Ideas:

Toolchain rules: some rules can be predefined to ease the creation of new toolchains and modification to existing toolchains.

  1. A cc_tool_config rule, used to:
    • define tool names and tool paths (or labels)
    • file set needed to run the tool
    • supported feature names
  2. cc_action_config rule:
    • maps action names to a tool name
    • allows overriding enabled feature names for the actions
    • a similar idea is covered by this proposal
  3. "feature set" rules
    • some predefined feature set rules like: unix_gcc, unix_clang, windows_msvc, windows_clang_cl (and maybe a "common" feature set)
    • users can write custom feature sets
    • already covered by this proposal
  4. A cc_toolchain_import rule:
    • allows importing cc libraries at toolchain level, very similar to cc_import
    • allows defining toolchain include directories
    • allows linking dynamic libs into the toolchain solib (when system_provided = False)
    • allows linking extra objects
    • when setting only dynamic / static libs, always do a dynamic / static link regardless of link mode.
  5. The cc_toolchain rule
    • Consumes targets of all the above rules, plus some extra settings (cpu, artifact patterns, sysroot, copts, cxxopts, linkopts, etc).
    • tool_configs, action_configs, feature_sets and toolchain_imports all accept multiple targets (as long as no conflict is detected).
    • allows overriding enabled feature names, globally.
    • allows "inheriting" another cc_toolchain rule, which would essentially use the ancestor's fields as default values.

Default toolchain and other common use cases:

  1. The default (auto-generated) toolchain
    • The default toolchain should auto detect the host compiler and generate a set of corresponding cc_tool_config targets.
    • Auto detect sysroot if needed
    • Uses a default set of cc_actions targets.
    • Selects the proper feature sets for the platform & compiler
  2. User replacing / adding tools: inherit the default toolchain and add a new cc_tool_config.
  3. User replacing / adding features: write a new "feature set", inherit the default toolchain and add to the "feature_sets" field.
  4. User adding a toolchain lib: inherit the default toolchain and add a new cc_toolchain_import.
  5. User replacing default / runtime libs: inherit the default toolchain, set linkopts for "no-default-libs" and add the new cc_toolchain_import targets for all the custom default libs.
  6. Setting up a separate toolchain: create a new cc_toolchain but do not inherit. Add custom configs / targets or the predefined targets when appropriate.

Build rules:

  1. cc_library
    • attr linkshared and linkstatic enforces only generating a shared / static library
    • attr dist_name: allows specifying a name for the library instead of having to use the default "libfoo" (e.g. a target called zlib can be named libz, not libzlib) - replacing the need for cc_binary(linkshared = 1)
  2. cc_binary
    • TBD

Copy link

github-actions bot commented Apr 6, 2024

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 Apr 6, 2024
Copy link

github-actions bot commented Jul 5, 2024

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please post @bazelbuild/triage in a comment here and we'll take a look. Thanks!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

3 participants