GradleFiles project scan should adhere to global root settings by sid-srini · Pull Request #9263 · apache/netbeans

sid-srini · 2026-03-12T15:33:46Z

Fixed GradleFiles scan for project files to honour the global scan root settings, if any.

Similar to that in projectapi SimpleFileOwnerQueryImpl.
GradleFiles needs to necessarily traverse the parent directory hierarchy to find the root project settings.
Further, the project detection needs to give precedence to maven projects also defined for the same directory, which has a similar hierarchy traversal requirement introduced in Java with Maven -> Java Frontend App + MOE is broken since Gradle support introduced #1280.

Fixed GradleFiles to find parentScript for kotlin script.

Added a unit test case for it in GradleFilesTest.

Fixed SimpleFileOwnerQueryImplementation to check for scan root dirs without allowing sibling dirs with the same name prefix.

Example: if scan root = "/tmp/app",
- then "/tmp/application" should not be allowed for scan;
- while "/tmp/app" and "/tmp/app/src" should remain allowed.

Fixed GradleFilesTest for File comparison on MacOS.

Needed due to the presence of "/private" prefix in the canonical path of unix temp folders.

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

are all your own work, and you have the right to contribute them.
are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

Was this PR correctly labeled, did the right tests run? When did they run?
Is this PR squashed?
Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

sid-srini · 2026-04-28T18:39:38Z

Hi @lkishalmi. Please review this when you get a chance. Thank you.

matthiasblaesing

In general makes sense to me. I left a few inline comments.

matthiasblaesing · 2026-05-05T18:18:31Z

+        for (String scanRoot : projectScanRoots) {
+            if (path.startsWith(scanRoot)
+                    && (path.length() == scanRoot.length()
+                    || path.charAt(scanRoot.length()) == '/'))


I assume this change intents to check that we don't get a substring match for the last component. So far so sensible, but will it work on windows paths?

PS: Also does not match logic in GradleFiles.Searcher#notWithinProjectScanRoots

Thanks @matthiasblaesing for your review and the close attention on these path comparison differences.

As noted in this class, in the separatePaths method, projectScanRoots store FileObject.getPath() values since getOwner supplies and compares with FileObject.getPath() strings. The FileObject instances canonicalize and always return / as the file-separator character, even on Windows.

This explicitly does not match the logic in GradleFiles.Searcher#notWithinProjectScanRoots which compares with File.separatorChar, because there GradleFiles uses File instance path comparisons instead. Thus, GradleFiles.Searcher#separatePaths notes that it needs to store both absolute and canonical paths in projectScanRoots.

I hope this clarifies the difference in approach.

matthiasblaesing · 2026-05-05T18:20:08Z

+        if (root != null && Utilities.isMac()) {
+            String absolutePath = root.getAbsolutePath();
+            if (absolutePath.startsWith("/private/")) {
+                return new File(absolutePath.substring(8));
+            }
+        }


This needs a comment. What is so special about the temp on mac and why does this break anything?

Thanks. I will add a comment in the source code, similar to the commit message and PR:

Needed due to the presence of "/private" prefix in the canonical path of unix temp folders.

On MacOS X 10.5 and all later macOS, the unix temporary folder "/tmp" is actually a symbolic link to "/private/tmp". This is the reason for the additional check since the test libraries and Java itself, set the tmpdir as "/tmp/..." but the path checks after canonicalization require testing against "/private/tmp/..."

I hope this clarifies this addition. Thanks.

matthiasblaesing · 2026-05-05T18:24:01Z

-                return isProjectCheck(parent, preferMaven);
+            FileObject parent = dir.getParent();
+            if (parent != null && GradleFiles.Searcher.searchPath(FileUtil.toFile(parent), "pom.xml") != null) { // NOI18N
+                return isProjectCheck(parent, false);


is preferMaven intentionally dropped here?

Thanks for noticing this.
Yes. The NetBeans IDE prompted me to replace preferMaven with false since it will always be so inside this block after the pre-emptive return on true at NbGradleProjectFactory.java:65. :)

I can revert it to refer to preferMaven itself here too. Please let me know. Thanks.

matthiasblaesing · 2026-05-05T18:54:35Z

+
+            Set<String> paths = null;
+            for (String split : joinedPaths.split(pathSeparator)) {
+                if ((split = split.trim()).isEmpty()) continue;


This will break when path contains leading or trailing spaces. Or is this just a copy from SimpleFileOwnerQueryImplementation? If so, please mark both places that they need to be synced on change.

Yes, this is copied from SimpleFileOwnerQueryImplementation as noted in the commit and PR. I will add a comment in both places to sync on change.

The trimming of leading and trailing spaces was prompted mostly from a usability perspective where user-input may add additional leading and trailing whitespace on paths. Guidance by Microsoft Windows is to avoid trailing whitespaces since it is unsupported by various UIs, even though it is supported by NTFS. These UIs end up trimming the input, causing problems when copy-pasting to another terminal.

However, since this may cause the breakage on legitimate (although uncommon) input, I will remove the trimming from here, as well as, from SimpleFileOwnerQueryImplementation, except to detect empty input. Thanks.

sid-srini · 2026-05-14T18:41:01Z

Thank you @matthiasblaesing for your review and feedback.
I've added a commit incorporating the feedback, fix and comments. Please review them. Thanks a lot.

Fixed GradleFiles scan for project files to honour the global scan root settings, if any. - Similar to that in projectapi SimpleFileOwnerQueryImpl. - GradleFiles needs to necessarily traverse the parent directory hierarchy to find the root project settings. - Further, the project detection needs to give precedence to maven projects also defined for the same directory, which has a similar hierarchy traversal requirement introduced in apache#1280. Fixed GradleFiles to find parentScript for kotlin script. - Added a unit test case for it in GradleFilesTest. Fixed SimpleFileOwnerQueryImplementation to check for scan root dirs without allowing sibling dirs with the same name prefix. - Example: if scan root = "/tmp/app", - then "/tmp/application" should not be allowed for scan; - while "/tmp/app" and "/tmp/app/src" should remain allowed. Fixed GradleFilesTest for File comparison on MacOS. - Needed due to the presence of "/private" prefix in the canonical path of unix temp folders. Signed-off-by: Siddharth Srinivasan <siddharth.srinivasan@oracle.com>

@test

Split the class into 2 classes: - GradleFilesWithScanRootTest now only runs the GradleFilesTest with the project limitScanRoot and forbiddenFolders properties set. - GradleFilesScanRootTest contains only the functionality of the scan-root and forbidden folders specifically. - Only a single JUnit @test can be used here since the static initialization in @before must be done once only. - This means the test root folder must not change between tests. - The actual gradle project is now a grandchild in the tree of the test root dir. - The original failure was due to the alternative placement of the tests temporary folder on linux which is not sufficiently nested within the OS tmp directory. Signed-off-by: Siddharth Srinivasan <siddharth.srinivasan@oracle.com>

1. Fixed the separatePaths() method in GradleFiles.Searcher and SimpleFileOwnerQueryImplementation to not trim() the split paths and only ignore completely blank paths. 2. Added implementation notes linking the logic and code of the above two classes so that changes made in the future are done to both. Signed-off-by: Siddharth Srinivasan <siddharth.srinivasan@oracle.com>

sid-srini · 2026-05-19T12:36:13Z

I've rebased this to the current master which includes post NB30 changes.
@matthiasblaesing - Kindly re-trigger the CI checks and review the changes. Thanks a lot.

mbien added Gradle [ci] enable "build tools" tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) labels Mar 12, 2026

apache locked and limited conversation to collaborators Mar 12, 2026

apache unlocked this conversation Mar 12, 2026

lkishalmi self-requested a review March 12, 2026 23:33

matthiasblaesing reviewed May 5, 2026

View reviewed changes

mbien added this to the NB31 milestone May 14, 2026

sid-srini added 3 commits May 19, 2026 18:03

sid-srini force-pushed the gradle-files-scan-root branch from af01fc0 to 53c3535 Compare May 19, 2026 12:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GradleFiles project scan should adhere to global root settings#9263

GradleFiles project scan should adhere to global root settings#9263
sid-srini wants to merge 3 commits into
apache:masterfrom
sid-srini:gradle-files-scan-root

sid-srini commented Mar 12, 2026

Uh oh!

sid-srini commented Apr 28, 2026

Uh oh!

matthiasblaesing left a comment

Uh oh!

matthiasblaesing May 5, 2026

Uh oh!

sid-srini May 14, 2026

Uh oh!

matthiasblaesing May 5, 2026

Uh oh!

sid-srini May 14, 2026

Uh oh!

matthiasblaesing May 5, 2026

Uh oh!

sid-srini May 14, 2026

Uh oh!

matthiasblaesing May 5, 2026

Uh oh!

sid-srini May 14, 2026 •

edited

Loading

Uh oh!

sid-srini commented May 14, 2026

Uh oh!

sid-srini commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sid-srini commented Mar 12, 2026

PR approval and merge checklist:

Uh oh!

sid-srini commented Apr 28, 2026

Uh oh!

matthiasblaesing left a comment

Choose a reason for hiding this comment

Uh oh!

matthiasblaesing May 5, 2026

Choose a reason for hiding this comment

Uh oh!

sid-srini May 14, 2026

Choose a reason for hiding this comment

Uh oh!

matthiasblaesing May 5, 2026

Choose a reason for hiding this comment

Uh oh!

sid-srini May 14, 2026

Choose a reason for hiding this comment

Uh oh!

matthiasblaesing May 5, 2026

Choose a reason for hiding this comment

Uh oh!

sid-srini May 14, 2026

Choose a reason for hiding this comment

Uh oh!

matthiasblaesing May 5, 2026

Choose a reason for hiding this comment

Uh oh!

sid-srini May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sid-srini commented May 14, 2026

Uh oh!

sid-srini commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sid-srini May 14, 2026 •

edited

Loading