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

Make the execution root structure match that of the runfiles tree #1262

Closed
kchodorow opened this issue May 13, 2016 · 15 comments
Closed

Make the execution root structure match that of the runfiles tree #1262

kchodorow opened this issue May 13, 2016 · 15 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@kchodorow
Copy link
Contributor

Followup to #848. Now we have execroot paths like:

output_base/execroot/local_repo/external/another_repo/foo/bar

but we want the execution root paths to look like:

output_base/execroot/another_repo/foo/bar

(Not exactly a bug, but we don't have an "improvement" category.)
@kchodorow kchodorow self-assigned this May 13, 2016
@kchodorow kchodorow added type: bug P2 We'll consider working on this in future. (Assignee optional) category: External repositories labels May 13, 2016
@kchodorow
Copy link
Contributor Author

One complication with this is what to do with outputs. For the main repository, you get the output of //foo:bar in:

bazel-out/local-fastbuild/bin/foo/bar

Bazel uses bazel-out/local-fastbuild/bin + execPath for these files, which doesn't work great when the execPath is ../another_repo/foo/bar. Once alternative is bazel-out/local-fastbuild/bin/external/another_repo/foo/bar, I'm going to talk to @gregestren about his plans for changing output dirs for configurations and see if this could piggypack on any of that.

@kchodorow kchodorow added this to the 0.4 milestone Jun 15, 2016
@kchodorow
Copy link
Contributor Author

Rolling back due to #1422.

@kchodorow kchodorow reopened this Jun 17, 2016
@kchodorow
Copy link
Contributor Author

Some breakages:

bazel-io pushed a commit that referenced this issue Jun 21, 2016
@kchodorow @philwo
Move execroot tree creation to its own class
52bc57c 
Part 1 of many for #1262, rolling forward.

--
MOS_MIGRATED_REVID=125334954
bazel-io pushed a commit that referenced this issue Jun 22, 2016
@kchodorow @philwo
Update ijar test
b17ebee 
Another part of rolling forward #1262.

--
MOS_MIGRATED_REVID=125481356
bazel-io pushed a commit that referenced this issue Jun 23, 2016
@kchodorow @lberki
Make execution root symlink builder handle PackageIdentifiers
33aada2 
Part of the rollforward for #1262.

--
MOS_MIGRATED_REVID=125562871
hermione521 pushed a commit to hermione521/bazel that referenced this issue Jun 27, 2016
@kchodorow @hermione521
Update ijar test
dfc6f26 
Another part of rolling forward bazelbuild#1262.

--
MOS_MIGRATED_REVID=125481356
hermione521 pushed a commit to hermione521/bazel that referenced this issue Jun 27, 2016
@kchodorow @hermione521
Make execution root symlink builder handle PackageIdentifiers
e74380d 
Part of the rollforward for bazelbuild#1262.

--
MOS_MIGRATED_REVID=125562871
bazel-io pushed a commit that referenced this issue Aug 12, 2016
@kchodorow @hermione521
Add the repository name as a parameter to the output path functions
f8a1ae6 
This doesn't do anything yet, it's in preparation for the execroot rearranging
change.  The execroot will have one bazel-out per repo, so it'll look like:

execroot/
  repo1/
    bazel-out/
      local-fastbuild/
        bin/
  repo2/
    bazel-out/
      local-fastbuild/
        bin/
        genfiles/
  repo3/
    bazel-out/
      local-fastbuild/
        testlogs/

and so on. Thus, any output path (getBinDirectory() & friends) needs to know
what the repo name is. This changes so many places in the code I thought it
would be good to do separately, then just flip the functionality in the
execroot-rearranging commit.

While I was poking around, I changed all of the refs I could from getPackageRelativeArtifact() to getBin/GenfilesArtifact(), so that 1) rule implementation don't have to know as much about roots and 2) they'll be more isolated from other output dir changes.

`bazel info` and similar just return roots for the main repository.

The only "change" is passing around a target label in the Java rules.

Continues work on #1262.

--
MOS_MIGRATED_REVID=129985336
bazel-io pushed a commit that referenced this issue Sep 9, 2016
@kchodorow @damienmg
Add an "is main repo" field to Roots
3fd3f1b 
This is part of prepping for #1262.

--
MOS_MIGRATED_REVID=132553178
bazel-io pushed a commit that referenced this issue Sep 9, 2016
@kchodorow
Set workspace name for sandbox dirs
8d2dd4b 
This is required for #1262, as execution roots for external repos will be
accessed via ../reponame under the exec root.

I'm trying to break up the change into several small CLs. This should have
no impact on the sandbox's behavior.

--
MOS_MIGRATED_REVID=132671034
bazel-io pushed a commit that referenced this issue Sep 9, 2016
@kchodorow
Add repository parameter to source artifact resolver
d07177d 
Needed for #1262. Doesn't do anything, yet, other than make the CL smaller.

--
MOS_MIGRATED_REVID=132671036
bazel-io pushed a commit that referenced this issue Sep 9, 2016
@kchodorow
Make the code that is using execRoot paths use execRoot paths
bd016c9 
Chipping away at making the big CL for #1262 smaller. Only runfiles paths
are different right now, so this makes getPathUnderExecRoot and getSourceRoot
return the same thing. Also corrected a couple places where
Label.EXTERNAL_PATH_PREFIX and Label.EXTERNAL_PACKAGE_NAME were being used
incorrectly.

--
MOS_MIGRATED_REVID=132671081
@damienmg damienmg reopened this Sep 20, 2016
@damienmg
Copy link
Contributor

Rollback so reopening :)

@kchodorow kchodorow reopened this Nov 18, 2016
@kchodorow
Copy link
Contributor Author

Change was rolled back, this shouldn't have been closed.

@kchodorow kchodorow modified the milestones: 0.5, 0.4 Nov 18, 2016
@kchodorow kchodorow mentioned this issue Nov 18, 2016
Closed
kchodorow added a commit to kchodorow/rules_go that referenced this issue Jan 20, 2017
@kchodorow
Get go rules working with execroot rearrangement
0e555cf 
I am getting ready to resubmit bazelbuild/bazel#1262,
which significantly changes execroot paths (external/foo becomes ../foo, essentially).
Without this patch, the execroot change messes up the symlink tree the go rules
create.

This compensates for the path change in a backwards-compatible way:
bazel test //... passes for me with old & patched versions of bazel.
@kchodorow kchodorow mentioned this issue Mar 9, 2017
Closed
bazel-io pushed a commit that referenced this issue Mar 31, 2017
@kchodorow @philwo
Symlink output directories to the correct directory name
1d9e1ac 
If the workspace directory is /path/to/my/proj and the name in the WORKSPACE
file is "floop", this will symlink the output directories to
output_base/execroot/floop instead of output_base/execroot/proj.

More prep for #1262, fixes #1681.

PiperOrigin-RevId: 151712384
bazel-io pushed a commit that referenced this issue Apr 4, 2017
@hlopko @damienmg
Automated g4 rollback of commit 1d9e1ac.
c413480 
*** Reason for rollback ***

Breaks //src/test/shell/integration:force_delete_output_test

*** Original change description ***

Symlink output directories to the correct directory name

If the workspace directory is /path/to/my/proj and the name in the WORKSPACE
file is "floop", this will symlink the output directories to
output_base/execroot/floop instead of output_base/execroot/proj.

More prep for #1262, fixes #1681.

PiperOrigin-RevId: 152126545
@hlopko
Copy link
Member

hlopko commented Apr 10, 2017

Hello Kristina, is this still expected to be fixed in 0.5?

This was referenced Apr 10, 2017
Closed
Merged
@kchodorow
Copy link
Contributor Author

No.

@damienmg damienmg modified the milestones: 0.6, 0.5 Apr 20, 2017
bazel-io pushed a commit that referenced this issue May 23, 2017
@kchodorow
Automated g4 rollback of commit c413480.
85ae190 
*** Reason for rollback ***

Roll forward of directory name change

*** Original change description ***

Automated g4 rollback of commit 1d9e1ac.

*** Reason for rollback ***

Breaks //src/test/shell/integration:force_delete_output_test

*** Original change description ***

Symlink output directories to the correct directory name

If the workspace directory is /path/to/my/proj and the name in the WORKSPACE
file is "floop", this will symlink the output directories to
output_base/execroot/floop instead of output_base/execroot/proj.

More prep for #1262, fixes #1681.

PiperOrigin-RevId: 156892980
@alexeagle alexeagle mentioned this issue Dec 4, 2017
Merged
@dslomov dslomov added external-repos-paths P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Jan 11, 2018
@alexeagle alexeagle mentioned this issue May 14, 2018
Closed
@dslomov dslomov added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed category: extensibility > external repositories type: bug P1 I'll work on this now. (Assignee required) labels Mar 21, 2019
@meisterT meisterT removed this from the 0.6 milestone May 12, 2020
@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@kchodorow @johnynek @meisterT @philwo @hlopko @damienmg @dslomov @sgowroji