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

Implement C++ Starlark API #4570

Closed
oquenchil opened this issue Feb 2, 2018 · 5 comments
Closed

Implement C++ Starlark API #4570

oquenchil opened this issue Feb 2, 2018 · 5 comments
Assignees
@oquenchil
Labels
P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@oquenchil
Copy link
Contributor

oquenchil commented Feb 2, 2018 •
edited
Loading

Description of the problem / feature request:

The API should expose to Skylark from Java the logic for the creation of compilation and linking actions.

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

The main problem to solve is inter-operability between different rules/languages that have dependencies on C++ code.

Design doc:https://docs.google.com/document/d/1cRRdHOPTTUXBbq9Cj9hk_WLnPqsGtAoQynYd7TKBQI8/edit?disco=AAAACkgMw7E
@oquenchil oquenchil added type: feature request P1 I'll work on this now. (Assignee required) category: rules > C++ labels Feb 2, 2018
@oquenchil oquenchil self-assigned this Feb 2, 2018
@hlopko hlopko mentioned this issue Feb 2, 2018
Closed
This was referenced Mar 5, 2018
Open
Merged
@hlopko hlopko mentioned this issue Mar 22, 2018
Closed
@hlopko hlopko mentioned this issue Apr 5, 2018
Open
@hlopko hlopko mentioned this issue May 2, 2018
Closed
@hlopko hlopko mentioned this issue May 14, 2018
Closed
@oquenchil oquenchil mentioned this issue Jun 20, 2018
Closed
This was referenced Jun 25, 2018
Closed
Open
@hlopko hlopko mentioned this issue Jul 4, 2018
Closed
bazel-io pushed a commit that referenced this issue Jul 19, 2018
@oquenchil
C++: Implements Skylark cc_common.compile()/link().
769f72e 
Working towards #4570.

RELNOTES:none
PiperOrigin-RevId: 205274676
@akonradi akonradi mentioned this issue Jul 30, 2018
Merged
@hlopko hlopko mentioned this issue Jul 31, 2018
Closed
@hlopko
Copy link
Member

hlopko commented Jul 31, 2018 •
edited
Loading

Bazel@HEAD now accepts --experimental_enable_cc_skylark_api that will enable the current WIP version of the API. It's only for experimenting at this point, we break it daily, and we will not support users using it yet. But feedback is more than welcomed.

@hlopko hlopko mentioned this issue Jul 31, 2018
Closed
@mfarrugi
Copy link

Is there a better way to traverse the API than print(dir(ctx.attr.cc_lib.cc))?

Would like to see if it better fits bazelbuild/rules_rust#108

werkt pushed a commit to werkt/bazel that referenced this issue Aug 2, 2018
@oquenchil
C++: Implements Skylark cc_common.compile()/link().
d4ee93f 
Working towards bazelbuild#4570.

RELNOTES:none
PiperOrigin-RevId: 205274676
bazel-io pushed a commit that referenced this issue Sep 20, 2018
@oquenchil
C++: Make cc_binary's linking go through CcLinkingHelper.
d37c675 
CcLinkingHelper has new functionality to support transitive linking. This is
not exposed to Skylark yet.

GitHub issue #4570

RELNOTES:none
PiperOrigin-RevId: 213824305
bazel-io pushed a commit that referenced this issue Sep 25, 2018
@hlopko
Automated rollback of commit d37c675.
0dfb94d 
*** Reason for rollback ***

b/116592380

*** Original change description ***

C++: Make cc_binary's linking go through CcLinkingHelper.

CcLinkingHelper has new functionality to support transitive linking. This is
not exposed to Skylark yet.

GitHub issue #4570

RELNOTES:none
PiperOrigin-RevId: 214451346
bazel-io pushed a commit that referenced this issue Sep 27, 2018
@oquenchil
C++: Make cc_binary's linking go through CcLinkingHelper.
93b1912 
CcLinkingHelper has new functionality to support transitive linking. This is
not exposed to Starlark yet.

GitHub issue #4570

This was rolled back because in the refactoring I started passing headers to the linking action that were not passed before. These headers have mistakes and cannot be pre-processed, so when they were passed as inputs to the linking action, the compilation action was executed whereas before it wasn't.

Fixed and added test to make sure the behavior is the same as before.

RELNOTES:none
PiperOrigin-RevId: 214785687
@hlopko hlopko added team-Rules-CPP Issues for C++ rules and removed team-Rules-CPP Issues for C++ rules category: rules > C++ labels Oct 11, 2018
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this issue Jan 31, 2019
@hlopko @weixiao-huang
Automated rollback of commit 924eee0.
7beca4a 
*** Reason for rollback ***

Breaks nightly

*** Original change description ***

C++: Remove more CcLinkParams

Tracking: bazelbuild#4570

RELNOTES:none
PiperOrigin-RevId: 229584100
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this issue Jan 31, 2019
@oquenchil @weixiao-huang
C++: Remove more CcLinkParams
99daa76 
Tracking: bazelbuild#4570

Fixes conversion bug from old API to new that was already present before the original CL but was only triggered with it.

RELNOTES:none
PiperOrigin-RevId: 229910560
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this issue Jan 31, 2019
@janakdr @weixiao-huang
Automated rollback of commit 56f2b2b.
e59a527 
*** Reason for rollback ***

b/123103413.

*** Original change description ***

C++: Remove more CcLinkParams

Tracking: bazelbuild#4570

Fixes conversion bug from old API to new that was already present before the original CL but was only triggered with it.

RELNOTES:none
PiperOrigin-RevId: 230094550
weixiao-huang pushed a commit to weixiao-huang/bazel that referenced this issue Jan 31, 2019
@oquenchil @weixiao-huang
C++: Remove precompiled files from CcCompilationHelper.
397096c 
Working towards bazelbuild#4570
RELNOTES:none
PiperOrigin-RevId: 230526082
@siddharthab siddharthab mentioned this issue Feb 2, 2019
Closed
bazel-io pushed a commit that referenced this issue Feb 7, 2019
@oquenchil
C++: Starts removing rule context from CcLinkingHelper
cffad55 
Working towards #4570

RELNOTES:none
PiperOrigin-RevId: 232843780
bazel-io pushed a commit that referenced this issue Feb 7, 2019
@oquenchil
C++: Do not look at nocopts in usePicForBinaries
58f4db9 
This required looking at the rule context and we want to stop doing that from behind the C++ Starlark API. This was only used by one target in the depot that is broken anyway. Temporarily in case this breaks anyone in Bazel, rather than using an incompatible flag, we can fix anyone broken by this using a rule feature. If no one complains, this replacement will be removed in a later release.

#4570

RELNOTES:none
PiperOrigin-RevId: 232848408
bazel-io pushed a commit that referenced this issue Feb 12, 2019
@oquenchil
C++: Completely removes ctx from compile.
4310989 
No instances of RuleContext left.

#4570

RELNOTES:none
PiperOrigin-RevId: 233590830
aherrmann added a commit to tweag/rules_haskell that referenced this issue Feb 13, 2019
@aherrmann
Update to Bazel 0.22 C++ API
5f53b20 
Bazel 0.22 contains further updates to the new C++ API:
bazelbuild/bazel#4570
It seems these removed access to the unmangled library names for C
library dependencies. On MacOS dynamic libraries contain the path to the
unmangled library in their "library name", which means that targets
linking against these dynamic libraries will reference the unmangled
target. We need to patch them to instead refer to the mangled target
relative to `@rpath`. As we no longer have access to the unmangled
targets, we can no longer easily patch the targets.

Furthermore, GHC generates intermediate dynamic libraries which will try
to load the unmangled libraries. Without them available, we cannot make
this step pass sucessfully.

We work around this by copying and patching the link library
dependencies to change their "library name" to the mangled name relative
to `@rpath`. Then we no longer need to patch the targets depending on
these libraries, as the linker will write the correct linking commands
into them.
aherrmann added a commit to tweag/rules_haskell that referenced this issue Feb 15, 2019
@aherrmann
Update to Bazel 0.22 C++ API
b53b0cd 
Bazel 0.22 contains further updates to the new C++ API:
bazelbuild/bazel#4570

It seems these updates removed access to the unmangled library names for
C library dependencies. On MacOS, dynamic libraries contain the path to
the unmangled library relative to the working directory in their
"library name", which means that targets linking against these dynamic
libraries will reference the unmangled target.

These references are invalid if a binary is executed in a different
working directory, and are unnecessarily long, which makes it easy to
exceed the MACH-O header size limit on MacOS.

Previously, rules_haskell would patch these load commands based on the
unmangled libraries to point to the mangled libraries under a single
RPATH entry. This shortened the load commands. Now that the unmangled
libraries are no longer available, this patching is performed in a
[linker wrapper script][osx_wrapper], similar to how Bazel itself
handles this.

Furthermore, GHC generates intermediate dynamic libraries which will try
to load the unmangled libraries. The wrapper script approach allows to
patch the intermediate library with additional RPATHs to avoid load
failures during GHC compilation steps. The Hazel target for the swagger2
library, for example, triggers this issue.

[osx_wrapper]: https://github.com/bazelbuild/bazel/blob/6aab2719d678057c4ef8edc3dfcad56803454732/tools/cpp/osx_cc_wrapper.sh.tpl.
aherrmann added a commit to tweag/rules_haskell that referenced this issue Feb 18, 2019
@aherrmann
Update to Bazel 0.22 C++ API
072bbba 
Bazel 0.22 contains further updates to the new C++ API:
bazelbuild/bazel#4570

It seems these updates removed access to the unmangled library names for
C library dependencies. On MacOS, dynamic libraries contain the path to
the unmangled library relative to the working directory in their
"library name", which means that targets linking against these dynamic
libraries will reference the unmangled target.

These references are invalid if a binary is executed in a different
working directory, and are unnecessarily long, which makes it easy to
exceed the MACH-O header size limit on MacOS.

Previously, rules_haskell would patch these load commands based on the
unmangled libraries to point to the mangled libraries under a single
RPATH entry. This shortened the load commands. Now that the unmangled
libraries are no longer available, this patching is performed in a
[linker wrapper script][osx_wrapper], similar to how Bazel itself
handles this.

Furthermore, GHC generates intermediate dynamic libraries which will try
to load the unmangled libraries. The wrapper script approach allows to
patch the intermediate library with additional RPATHs to avoid load
failures during GHC compilation steps. The Hazel target for the swagger2
library, for example, triggers this issue.

[osx_wrapper]: https://github.com/bazelbuild/bazel/blob/6aab2719d678057c4ef8edc3dfcad56803454732/tools/cpp/osx_cc_wrapper.sh.tpl.
@hlopko hlopko mentioned this issue Apr 5, 2019
Closed
bazel-io pushed a commit that referenced this issue Apr 29, 2019
@oquenchil @copybara-github
C++: Adds more tests for C++ Starlark API
78cf266 
Also exposes additional methods in the API:
merge_compilation_outputs and create_compilation_outputs

#4570

https://docs.google.com/document/d/1cRRdHOPTTUXBbq9Cj9hk_WLnPqsGtAoQynYd7TKBQI8/edit?disco=AAAACkgMw7E

RELNOTES:none
PiperOrigin-RevId: 245731814
bazel-io pushed a commit that referenced this issue Apr 30, 2019
@oquenchil @copybara-github
C++: Remove language documentation.
c9ae961 
It's not yet clear whether the API is enough for languages other than C++.
Therefore, remove language parameter documentation from the API.

#4570

RELNOTES:none
PiperOrigin-RevId: 245910010
bazel-io pushed a commit that referenced this issue Apr 30, 2019
@oquenchil @copybara-github
C++: Give error instead of crash with wrong extensions
5b76c1b 
The code was crashing on preconditions when passing wrong extensions for srcs
and hdrs attribute in the C++ compile() API.

#4570

RELNOTES:none
PiperOrigin-RevId: 245925608
iirina pushed a commit to iirina/bazel that referenced this issue Apr 30, 2019
@oquenchil @iirina
C++: Remove language documentation.
8355d20 
It's not yet clear whether the API is enough for languages other than C++.
Therefore, remove language parameter documentation from the API.

bazelbuild#4570

RELNOTES:none
PiperOrigin-RevId: 245910010
bazel-io pushed a commit that referenced this issue Apr 30, 2019
@oquenchil @copybara-github
C++: Fix crash with bad filetypes in compilation outputs
da2d8d9 
#4570

RELNOTES:none
PiperOrigin-RevId: 245947851
bazel-io pushed a commit that referenced this issue Apr 30, 2019
@oquenchil @copybara-github
C++: Adds tests for transitive link and fixes bugs
a2e6863 
Bugs fixed:
 - Creating static libraries during transitive link
 - Could not return None from linking_outputs' executable or library_to_link
 - Null pointer exception when not passing compilation_outputs to link()

#4570

RELNOTES:none
PiperOrigin-RevId: 245956660
bazel-io pushed a commit that referenced this issue Apr 30, 2019
@oquenchil @copybara-github
C++: Remove whitelisting of Starlark API for Bazel
0b80b9c 
#4570

RELNOTES:C++ Starlark API for compilation and linking is no longer whitelisted
PiperOrigin-RevId: 245964332
@nathaniel-brough nathaniel-brough mentioned this issue May 11, 2019
Closed
@oquenchil
Copy link
Contributor Author

Sandwich is ready:
https://docs.bazel.build/versions/master/skylark/lib/cc_common.html#compile

@excitoon
Copy link
Contributor

excitoon commented Jul 17, 2019 •
edited
Loading

Hey @oquenchil . How to pass data to LinkBuildVariables of cc_common.link? For example, interface_library name?

CcModule.java:290

        /* interfaceLibraryBuilder= */ null,
        /* interfaceLibraryOutput= */ null,

Currently it is being calculated to "ignored", producing command line like (Windows):
/IMPLIB:ignored

Repro case is here:
https://groups.google.com/forum/#!topic/bazel-discuss/I-yvg7xbbG0

@excitoon
Copy link
Contributor

Also, dynamic libraries in create_linking_context_from_compilation_outputs are not being generated on Windows:
CcModule.java:1435

            .setShouldCreateDynamicLibrary(
                !disallowDynamicLibraries
                    && !featureConfiguration
                        .getFeatureConfiguration()
                        .isEnabled(CppRuleClasses.TARGETS_WINDOWS))

luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
Automated rollback of commit 56f2b2bbae39d432609afa210db97d6c5669319a.
de778af 
    *** Reason for rollback ***

    b/123103413.

    *** Original change description ***

    C++: Remove more CcLinkParams

    Tracking: bazelbuild/bazel#4570

    Fixes conversion bug from old API to new that was already present before the original CL but was only triggered with it.

    RELNOTES:none
    PiperOrigin-RevId: 230094550
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
C++: Remove more CcLinkParams
8b68bbe 
    Tracking: bazelbuild/bazel#4570

    Fixes conversion bug from old API to new that was already present before the original CL but was only triggered with it.

    RELNOTES:none
    PiperOrigin-RevId: 229910560
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
Automated rollback of commit 924eee01460bbcd7de281f726f630d1e1e8889b9.
cbc3d7a 
    *** Reason for rollback ***

    Breaks nightly

    *** Original change description ***

    C++: Remove more CcLinkParams

    Tracking: bazelbuild/bazel#4570

    RELNOTES:none
    PiperOrigin-RevId: 229584100
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
C++: Remove more CcLinkParams
660e15a 
    Tracking: bazelbuild/bazel#4570

    RELNOTES:none
    PiperOrigin-RevId: 229181236
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
C++: Remove old params from API
68c227c 
    bazelbuild/bazel#4570

    RELNOTES:none
    PiperOrigin-RevId: 227522672
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
C++: Remove whitelist flag for new API
dc12090 
    Remove the flag for parts of the API that are stable.

    bazelbuild/bazel#4570

    RELNOTES: None.
    PiperOrigin-RevId: 227516883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@oquenchil oquenchil

Labels
P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@hlopko @excitoon @mfarrugi @oquenchil and others