intellij
intellij copied to clipboard
Make ArtifactLocation.getExecutionRootRelativePath() return Path inst…
Make ArtifactLocation.getExecutionRootRelativePath() return Path instead of String.
Update call-sites accordingly:
- Fix ArtifactLocationDecoderImpl.outputArtifactFromExecRoot being broken on Windows
- In the RemoteOutputArtifact always convert a path to canonical (with '/' on any OS) as remoteOutputArtifacts are keyed by OutputArtifact.getKey(), which always uses '/'.
- Trivial changes in all other places.
Checklist
- [x] I have filed an issue about this change and discussed potential changes with the maintainers.
- [ ] I have received the approval from the maintainers to make this change.
- [x] This is not a stylistic, refactoring, or cleanup change.
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See the Contributions section in the README for more details.
Discussion thread for this change
Issue numbers: https://github.com/bazelbuild/intellij/issues/113 - this fixes one of the biggest problems on Windows - most of External Libraries disappearing when doing "Sync Project with BUILD files", specifically when Working Set is empty/insufficient.
It was also discussed in https://github.com/bazelbuild/intellij/issues/490#issuecomment-454383837 - there it was assumed that the problem is in the missing/broken jdeps files, which isn't correct as of now - jdeps files are correct, but we fail to read them because of paths issue in ArtifactLocationDecoderImpl.
There people have come with various workarounds (e.g. creating new branch helps, as it causes Working Set to be null (not empty!), which results in all targets being built and all target dependencies being added to External Libraries).
There are also at least 3 workaround-ish PR https://github.com/jesseschalken/intellij/commit/3b805f7f397856e08d723a46eca74cfa27418441 and https://github.com/gergelyfabian/intellij/commit/8e1bfdacb2f99da77582d736a0f5127d473be71f which "solve" the issue by doing full build every time (which is non-scalable).
https://github.com/bazelbuild/intellij/pull/1204 - tries to solve the actual problem in ArtifactLocationDecoderImpl, but in a bit of a hacky way.
There are also probably some duplicate issues (e.g. https://github.com/bazelbuild/intellij/issues/2277). I'll try to link them in the comments.
Description of this change
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.
Sorry for the delay. I was going to try this again, but i can't reproduce the issue described in #2277 any more
Thank you for contributing to the IntelliJ repository! This pull request has been marked as stale since it has not had any activity in the last 6 months. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-review". Please reach out to the triage team (@bazelbuild/triage) if you think this PR is still relevant or you are interested in getting the PR merged.
This pull request has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!