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

Stop loading packages in CppConfiguration #6072

Closed
hlopko opened this issue Sep 4, 2018 · 5 comments
Closed

Stop loading packages in CppConfiguration #6072

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

Comments

@hlopko
Copy link
Member

hlopko commented Sep 4, 2018

Tracking the effort to finally remove package loading from CppConfigurationLoader and CppConfiguration.

@hlopko hlopko added type: feature request P1 I'll work on this now. (Assignee required) category: rules > C++ team-Rules-CPP Issues for C++ rules labels Sep 4, 2018
@hlopko hlopko self-assigned this Sep 4, 2018
bazel-io pushed a commit that referenced this issue Sep 4, 2018
This is a preparation for a cl removing package loading from CppConfiguration.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 211453414
bazel-io pushed a commit that referenced this issue Sep 4, 2018
#6072.

RELNOTES: None.
PiperOrigin-RevId: 211455462
bazel-io pushed a commit that referenced this issue Sep 4, 2018
#6072.

RELNOTES: None.
PiperOrigin-RevId: 211464711
bazel-io pushed a commit that referenced this issue Sep 5, 2018
Currently FdoProfiler is provided by the cc_toolchain separately. This causes
problems to custom skylark rules that store the implicit attribute as
_cc_toolchain, and native rules store it as ':cc_toolchain'. And FdoProvider was
expected to be provided by the ':cc_toolchain'.

Since FdoProvider is used together with the CcToolchainProvider, I'm merging
them together.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 211635916
bazel-io pushed a commit that referenced this issue Sep 7, 2018
Apart from slight moving of blocks of code nearer to where they are used, this cl does one major thing.

Before:
When we didn't find a label in cc_toolchain_suite.toolchains satisfying '--cpu|--compiler', we iterated over the CROSSTOOL and selected a CToolchain 'foo'. Then we used that 'foo.targetCpu|foo.compiler' to get the label from cc_toolchain_suite.toolchains to get the cc_toolchain 'bar'. So we used (foo, bar).

After:
After we find 'bar', and bar.toolchain_identifier is set, we select the CToolchain 'baz' from the CROSSTOOL matching that toolchain_identifier. Baz doesn't necessarily have to be the same as foo. So we use (foo, baz).

#6072.

RELNOTES: None.
PiperOrigin-RevId: 211981401
bazel-io pushed a commit that referenced this issue Sep 13, 2018
They should be used during toolchain selection (will happen in unknown commit). I
think it's an oversight that they are configurable now.

#6072

RELNOTES: None
PiperOrigin-RevId: 212791669
bazel-io pushed a commit that referenced this issue Sep 13, 2018
…ain in cc_toolchain

Rather re-select the toolchain for yourself. The next step will be to stop relying on CROSSTOOL in the CppConfiguration.
#6072.

RELNOTES: None.
PiperOrigin-RevId: 212813981
bazel-io pushed a commit that referenced this issue Sep 17, 2018
As a result, I could rewire CToolchain selection in
CppConfigurationLoader.selectCcToolchainLabel to
use the same entry point method as we do in the
CppConfigurationLoader.createParameters and in CcToolchain.create, and simplify
the code a bit.

#6072

RELNOTES: None.
PiperOrigin-RevId: 213258694
bazel-io pushed a commit that referenced this issue Sep 21, 2018
…lchain

Use CcSkyframeSupportFunction instead.

#6072

RELNOTES: None.
PiperOrigin-RevId: 214004051
bazel-io pushed a commit that referenced this issue Sep 21, 2018
*** Reason for rollback ***

Suspected breakage of perf regtest

*** Original change description ***

Do not depend on CppConfiguration to provide CROSSTOOL file to cc_toolchain

Use CcSkyframeSupportFunction instead.

#6072

RELNOTES: None.
PiperOrigin-RevId: 214043149
bazel-io pushed a commit that referenced this issue Sep 25, 2018
…lchain

Use CcSkyframeSupportFunction instead.

#6072

This is a rollforward of unknown commit after fixing loading CROSSTOOL from different package roots.

RELNOTES: None.
PiperOrigin-RevId: 214394291
bazel-io pushed a commit that referenced this issue Sep 26, 2018
This is a preparation for a change moving CcToolchainProvider creation from
CcToolchain to CcToolchainSuite.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 214589270
bazel-io pushed a commit that referenced this issue Sep 26, 2018
This is a preparation for moving CcToolchainProvider creation to the
cc_toolchain_suite. This cl just moves code around to make the diff of the
followup cl smaller.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 214624986
@ob
Copy link
Contributor

ob commented Sep 26, 2018

Is this going to help with objc_library not loading so many more dependencies than cc_library?

@hlopko
Copy link
Member Author

hlopko commented Sep 27, 2018

Nope, it will only help with not loading C++ toolchain in the loading phase, only in the analysis phase (which is a prerequisite for migrating C++ rules to platforms). I'm not sure I know which issue are you referring to?

bazel-io pushed a commit that referenced this issue Sep 27, 2018
This provider encapsulates everything that is cc_toolchain-rule specific and
contributes to the construction of CcToolchainProvider. In the followup cl,
cc_toolchain will be modified to provide CcToolchainAttributesProvider, and
cc_toolchain_suite will get the provider and construct the CcToolchainProvider
from it.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 214762144
@ob
Copy link
Contributor

ob commented Sep 27, 2018

I'm talking about this:

$ cat test.sh 
#!/bin/sh

rm -rf tmp_workspace
mkdir tmp_workspace
touch WORKSPACE
cat > BUILD <<EOF
objc_library(
  name = 'objc',
  srcs = ['a.m'],
)

cc_library(
  name = 'cc',
  srcs = ['a.c'],
)
EOF
# discard output about starting the daemon
bazel version >/dev/null 2>&1

echo objc_library implicit dependencies
bazel query --host_deps --implicit_deps 'deps(//:objc)' | sort >objc_deps
echo cc_library implicit dependencies
bazel query --host_deps --implicit_deps 'deps(//:cc)' | sort >cc_deps
echo objc_lib has `wc -l objc_deps` dependencies
echo cc_lib has `wc -l cc_deps` dependencies
diff -u cc_deps objc_deps

which produces

objc_library implicit dependencies
cc_library implicit dependencies
objc_lib has 577 objc_deps dependencies
cc_lib has 54 cc_deps dependencies

Full output here

bazel-io pushed a commit that referenced this issue Oct 1, 2018
…c_toolchain_label_from_crosstool_proto

Users commonly hit a problem when migrating their toolchains for this flag. Adding a paragraph to make them aware of the solution.

#6072.

RELNOTES: None.
PiperOrigin-RevId: 215186089
bazel-io pushed a commit that referenced this issue Oct 10, 2018
This way, we don't have to do any package loading in the CppConfiguration
(yay!). cc_toolchain now provides CcToolchainAttributesProvider, a lightweight
representation of its attributes, and cc_toolchain_suite will select one and use
it to create the full CcToolchainProvider.

This cl should be backwards compatible under normal use, but there are some
behavioral differences:

* Before, cc_toolchain_suite was not analyzed
* Before, only the selected cc_toolchain was analyzed
* Now, cc_toolchain_suite is analyzed
* Now, all cc_toolchains of the cc_toolchain_suite are analyzed

#6072

RELNOTES: CppRules: All cc_toolchains depended on from cc_toolchain_suite.toolchains are now analyzed when not using platforms in order to select the right cc_toolchain.
PiperOrigin-RevId: 216605275
@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
bazel-io pushed a commit that referenced this issue Oct 26, 2018
This way, we don't have to do any package loading in the CppConfiguration
(yay!). cc_toolchain now provides CcToolchainAttributesProvider, a lightweight
representation of its attributes, and cc_toolchain_suite will select one and use
it to create the full CcToolchainProvider.

This cl should be backwards compatible under normal use, but there are some
behavioral differences:

* Before, cc_toolchain_suite was not analyzed
* Before, only the selected cc_toolchain was analyzed
* Before, cc_toolchain_alias returned cc_toolchain target (observable from aspects)
* Now, cc_toolchain_suite is analyzed
* Now, all cc_toolchains of the cc_toolchain_suite are analyzed
* Before, cc_toolchain_alias returns cc_toolchain_suite target (observable from aspects)

#6072

This is a roll-forward of 3aedb2f after adding an incompatible flag.

RELNOTES: CppRules: All cc_toolchains depended on from cc_toolchain_suite.toolchains are now analyzed when not using platforms in order to select the right cc_toolchain.
PiperOrigin-RevId: 218865033
bazel-io pushed a commit that referenced this issue Nov 20, 2018
This way, we don't have to do any package loading in the CppConfiguration
(yay!). cc_toolchain now provides CcToolchainAttributesProvider, a lightweight
representation of its attributes, and cc_toolchain_suite will select one and use
it to create the full CcToolchainProvider.

This cl should be backwards compatible under normal use, but there are some
behavioral differences:

* Before, cc_toolchain_suite was not analyzed
* Before, only the selected cc_toolchain was analyzed
* Before, cc_toolchain_alias returned cc_toolchain target (observable from aspects)
* Now, cc_toolchain_suite is analyzed
* Now, all cc_toolchains of the cc_toolchain_suite are analyzed
* Before, cc_toolchain_alias returns cc_toolchain_suite target (observable from aspects)

Progress on #6072
Fixes #6537

RELNOTES: None.
PiperOrigin-RevId: 222289686
@hlopko
Copy link
Member Author

hlopko commented Nov 28, 2018

I failed to add the issue number to some of my late commits. Here they come:

@hlopko
Copy link
Member Author

hlopko commented Nov 28, 2018

And with this I declare the issue fixed.

@hlopko hlopko closed this as completed Nov 28, 2018
bazel-io pushed a commit that referenced this issue Nov 28, 2018
Also remove special cache for crosstools, they are now fully managed by the
Skyframe caches in the same way as any other file is managed.

#6072

RELNOTES: None.
PiperOrigin-RevId: 223147737
bazel-io pushed a commit that referenced this issue Nov 29, 2018
…ction

#6072.

RELNOTES: None
PiperOrigin-RevId: 223312270
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

2 participants