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

Full sync on build file changes #78

Open
wants to merge 17 commits into
base: wix-master
Choose a base branch
from

Conversation

mildagle
Copy link

Checklist

  • 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.
  • 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 number: <please reference the issue number or url here>

Description of this change

Copy link

@ittaiz ittaiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise change looks good.
Didn't think deeply on whether the aspect really catches all cases this way but I trust you did.
Left one small comment on File vs Path

}
});
return ImmutableSet.copyOf(sourceFiles);
}

private static void addBuildFile(Set<File> sourceFiles, String workSpaceRootPath,
String buildFilePath) {
File buildFile = new File(workSpaceRootPath, buildFilePath);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use Files and Path here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honest question, why would that be better? Or I don't understand what you mean

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because Path is the more idiomatic tool when writing Java code.
Part of the challenge in Java is that you have all of the old APIs forever so the motivation to move to the newer better abstractions is low.

@WixBuildServer WixBuildServer force-pushed the wix-master branch 2 times, most recently from 5215b08 to 380a8e9 Compare February 1, 2023 05:05
ZachiNachshon and others added 15 commits February 4, 2023 05:06
…47)

* only filters labels according to parent packages for LanguageClass.C
This is to keep supporting CPP need while also supporting aggregate targets
More info:
https://bazelbuild.slack.com/archives/CM8JQCANN/p1566481600003600
Filter was deliberately added to fix an issue with C++ headers.
The original bug was that opening the BUILD file for foo/bar.h caused it to open baz/qux/BUILD because there’s a cc_library target in baz/qux that includes foo/bar.h in its headers. The source-to-target look up queries the index for the target during the last sync which compiled the source file. In this case, it was the cc_library target in baz/qux.
So the fix was to ensure that the BUILD file is always for the parent package (foo/bar)
Don't use ijars as they break source navigation

- ignore java_import /_ijar/ as its original location is not know
- replace -ijar with sibling full jar
- replace -hjar with sibling full jar

https://youtrack.jetbrains.com/issue/SCL-16975/Navigation-to-attached-Scala-sources-for-interface-jar-does-not-work
Add support for scala source attachment
Add source root detection for Java and Scala source jars. Detection is required when dire4ctory structure in source jar has deeper structure than source packages.

Source attachement changes for 2022.3 support
`idea.plugin.protoeditor` needs to be aware of source roots of proto
libraries

Add proto files to target sources
Via running bloop and BSP

Fast test supports ITs
Location substitution supports runpath(s) in addition to location(s)
Still includes a hack of the env variable of BUILD_TOOL which we need to see how to remove but at least a smaller hack than before

Full Compile on proto file changes

don't update sources after proto change already exists
@WixBuildServer WixBuildServer force-pushed the wix-master branch 5 times, most recently from 8b5dcf2 to 5e997b6 Compare July 17, 2024 05:01
@WixBuildServer WixBuildServer force-pushed the wix-master branch 3 times, most recently from 0bfb111 to 0820309 Compare July 24, 2024 05:01
@WixBuildServer WixBuildServer force-pushed the wix-master branch 4 times, most recently from 63a8008 to 2966d75 Compare July 31, 2024 05:01
@WixBuildServer WixBuildServer force-pushed the wix-master branch 5 times, most recently from b06f7c9 to 00c4218 Compare August 9, 2024 05:01
@WixBuildServer WixBuildServer force-pushed the wix-master branch 3 times, most recently from 760b14f to 8f7195f Compare August 17, 2024 05:01
@WixBuildServer WixBuildServer force-pushed the wix-master branch 3 times, most recently from d75934d to 55c12f1 Compare August 22, 2024 05:01
@WixBuildServer WixBuildServer force-pushed the wix-master branch 5 times, most recently from c4688fc to 7d1319b Compare September 3, 2024 05:02
@WixBuildServer WixBuildServer force-pushed the wix-master branch 2 times, most recently from 2686c19 to 5b80ab4 Compare September 5, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants