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

Treat existence of managed directories as a part of repository dirtiness. #8564

Closed

Conversation

irengrig
Copy link
Contributor

@irengrig irengrig commented Jun 5, 2019

…ess:

- If managed directory is declared for the external repository and does not exist (probably was deleted by user), make external repository dirty and re-fetch it.
- Add tests to demonstrate the added behavior to ManagedDirectoriesBlackBoxTest.
- Closes bazelbuild#8487.
@irengrig irengrig requested a review from hlopko as a code owner June 5, 2019 16:11
@irengrig irengrig removed the request for review from hlopko June 5, 2019 16:11
@irengrig irengrig added the WIP label Jun 5, 2019
@bazel-io bazel-io closed this in 6efc5b7 Jun 6, 2019
@Globegitter
Copy link

@irengrig will this be cherry picked into the 0.26 and/or 0.27 release?

@irengrig
Copy link
Contributor Author

irengrig commented Jun 7, 2019

Asked for cherry-pick into 0.27, as 0.26.1 is already out.

@dslomov
Copy link
Contributor

dslomov commented Jun 7, 2019

@alexeagle do we absolutely need this cherry-picked for 0.27? Or is it ok for this fix to be in 0.28?

@laurentlb
Copy link
Contributor

When you think a bug should be a release blocker, please add the release blocker label (if you can) and cc the release manager (the person assigned to the release tracking issue).

The bug was filed 9 days ago. If it was critical, it would have been useful to mention it earlier.

@alexeagle
Copy link
Contributor

We haven't been able to adopt the managed_directories feature, so this bug is a show-stopper for us. So it is quite important for us to have it fixed in next release, yes. Is it risky/difficult to cherry-pick?

reasoning:
Node developers are accustomed to problems with their third-party deps, and their instinctive reaction is to delete them and re-install. If you do this after Bazel has installed the deps, that deletion destroys the BUILD files we wrote, and then the re-install does not re-create them. The error doesn't give any hint that you need to bazel clean --expunge to fix the wrong state of the managed directory, so users get stuck. For now we don't recommend they use the feature because it's so hard to recover from this common situation. This means they still have to install all of their dependencies twice which makes Bazel seem much slower than existing tooling.

@dslomov
Copy link
Contributor

dslomov commented Jun 10, 2019

@alexeagle what if we fix it in 0.28 (July) release?

@alexeagle
Copy link
Contributor

Then it is another month before we can finish rolling out the feature, which makes me worry we'll miss other problems that require some dogfooder adoption.
In the meantime we have already switched our documentation and quickstart to managed directories so we have users getting really confused - have seen a few already and we anticipate more.

@laurentlb
Copy link
Contributor

Related discussion around release frequency: https://groups.google.com/d/msg/bazel-dev/Ez4B3N_YEQY/PVzVJZL3BAAJ

@dslomov
Copy link
Contributor

dslomov commented Jun 12, 2019

@laurentlb what do you think about cherry-picking this given @alexeagle comments?

laurentlb pushed a commit that referenced this pull request Jun 12, 2019
…ess.

- If managed directory is declared for the external repository and does not exist (probably was deleted by user), make external repository dirty and re-fetch it.
- Add tests to demonstrate the added behavior to ManagedDirectoriesBlackBoxTest.
- Closes #8487.

Closes #8564.

PiperOrigin-RevId: 251882207
irengrig added a commit to irengrig/bazel that referenced this pull request Jun 18, 2019
…ess.

- If managed directory is declared for the external repository and does not exist (probably was deleted by user), make external repository dirty and re-fetch it.
- Add tests to demonstrate the added behavior to ManagedDirectoriesBlackBoxTest.
- Closes bazelbuild#8487.

Closes bazelbuild#8564.

PiperOrigin-RevId: 251882207
irengrig added a commit to irengrig/bazel that referenced this pull request Jul 15, 2019
…ess.

- If managed directory is declared for the external repository and does not exist (probably was deleted by user), make external repository dirty and re-fetch it.
- Add tests to demonstrate the added behavior to ManagedDirectoriesBlackBoxTest.
- Closes bazelbuild#8487.

Closes bazelbuild#8564.

PiperOrigin-RevId: 251882207
@irengrig irengrig deleted the deleted-managed-directories branch October 14, 2019 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Managed directories should rerun repository rule if directory is deleted or timestamp changes
6 participants