Spring Tool Suite
  1. Spring Tool Suite
  2. STS-2175

Avoid changing the <classpathenty>s in .classpath of the main project (if not requested) when importing a Gradle multiproject

    Details

    • Type: Bug Bug
    • Status: Resolved Resolved
    • Priority: Major Major
    • Resolution: Complete
    • Affects Version/s: 2.8.0.M2
    • Fix Version/s: 2.9.0.M1
    • Component/s: GRADLE
    • Labels:
      None

      Description

      Assume you have a Gradle multiproject and that it is under version control.
      Suppose this project is already an Eclipse project and all the Eclipse metadata files (.project, .classpath, .settings folder, etc.) are under version control.
      You check it out in a folder and then use the STS Gradle Import Wizard to import in a new repository.
      Suppose you UNCHECK all the following: run before, run after and create resource filters.
      You do that because you know that all the necessary data are already there, because the whole thing is under version control: so, you just want to "mount" the main project and all the subprojects into the workspace. (note: ideally, you may use the standard Import Existing Project into workspace, but you can't because the main project, which is at the root of the Gradle multiproject, is hiding the subprojects).

      After the import process has finished, even if you chose not to do anything special, you'll see some outgoing changes in the workspace:
      .settings/com.springsource.sts.gradle.core.prefs for all the subprojects: see STS-2174
      .settings/com.springsource.sts.gradle.core.import.prefs of the main project: see STS-2174
      .classpath of the main project: the order of the classpath entries related to the subprojects is not predictable

      I mean, repeating the project import over time shows that the <classpathentry>s in the .classpath file are changed every time: the content is the same, but the order of the different entries can change.

      Why the import wizard changes that file if I unchecked the execution of all the Eclipse Gradle plugin tasks? I would expect the wizard to just "mount" the projects, without touching the .classpath of the main project. This would be the best way to solve this problem, because if the .classpath is there (and is under version control), I assume I want it to be like that.

      If changing is unavoidable, at least please make the import wizard generate those <classpathentry>s in a predictable order: they may reflect the order in which dependencies are declared in the Gradle build script or, at least, some other sorting criteria (alphabetical on the subprojects name, for instance).

        Activity

        Hide
        Kris De Volder (c) added a comment -

        Actually running the eclipse tasks is only a kind of 'hack'. We use this so that some stuff that is generated by the eclipse plugin for Gradle, but not yet available through the tooling API is setup before the wizard makes its own changes.

        I.e. the wizard will always make changes, setting those options is for when you use eclipse features such as those related to groovy, aspectj, WTP etc. which are not yet supported through the tooling API.

        The wizard will always take control over the Java related aspects of the project.

        However, there is an issue open for providing a more 'hands-off' mode of working with the tools.

        https://issuetracker.springsource.com/browse/STS-2085

        I do however agree that, in any case, ramdomly changing the order of classpath entries is undesirable.

        So let's make this issue about fixing that instead of considering it a duplicate of STS-2085.

        Show
        Kris De Volder (c) added a comment - Actually running the eclipse tasks is only a kind of 'hack'. We use this so that some stuff that is generated by the eclipse plugin for Gradle, but not yet available through the tooling API is setup before the wizard makes its own changes. I.e. the wizard will always make changes, setting those options is for when you use eclipse features such as those related to groovy, aspectj, WTP etc. which are not yet supported through the tooling API. The wizard will always take control over the Java related aspects of the project. However, there is an issue open for providing a more 'hands-off' mode of working with the tools. https://issuetracker.springsource.com/browse/STS-2085 I do however agree that, in any case, ramdomly changing the order of classpath entries is undesirable. So let's make this issue about fixing that instead of considering it a duplicate of STS-2085 .
        Hide
        Mauro Molinari added a comment -

        Actually, STS-2085 would not be the case. In my case the needed Eclipse files (.project, .classpath, .settings...) are already there, because they have been stored in the SCM. All I would need is just creating the necessary references in workspace metadata to "link" the projects in the workspace. I don't know if STS-2085 is exactly about this or not... from the description I would say it is asking to also generate the .project, .classpath, etc. files within the project folder.

        Anyway, even if the wizard takes control over the Java related aspects of the project, it would be great if it could detect if a change to the .classpath file of the projects is really needed or not. If not, it shouldn't touch it (or, at least, it shouldn't change the order of classpath entries, as you said).

        Show
        Mauro Molinari added a comment - Actually, STS-2085 would not be the case. In my case the needed Eclipse files (.project, .classpath, .settings...) are already there, because they have been stored in the SCM. All I would need is just creating the necessary references in workspace metadata to "link" the projects in the workspace. I don't know if STS-2085 is exactly about this or not... from the description I would say it is asking to also generate the .project, .classpath, etc. files within the project folder. Anyway, even if the wizard takes control over the Java related aspects of the project, it would be great if it could detect if a change to the .classpath file of the projects is really needed or not. If not, it shouldn't touch it (or, at least, it shouldn't change the order of classpath entries, as you said).
        Hide
        Kris De Volder (c) added a comment -

        Right STS-2085 may not be exactly 'do nothing'. It be more like use the files generated by the eclipse tasks, but in the import wizard, this 'hands off' mode would amount to do nothing if you elected to not run the tasks. It deserves more thought.

        I will keep in mind your type of use case, which is import projects 'as is'.

        However, I imagine even so, there will still be some need to update the eclipse config files now and then, either via the STS tooling calling on the tooling API, or running some gradle tasks to modify the project configs.

        I'm thinking a 'hands off' mode would mostly do nothing unless you, the user, invoked some specific menu option or command to update something explicitly.

        But I'm starting to see that 'hands-off' versus 'automated' may be a different thing than 'generate only mode' versus 'query the tooling API mode'.

        The generate option is more about how it refreshes state whereas the 'hands-off' be more about the when (and who or what triggers refreshes).

        So you are right... that would be a separate concern from STS-2085.

        Show
        Kris De Volder (c) added a comment - Right STS-2085 may not be exactly 'do nothing'. It be more like use the files generated by the eclipse tasks, but in the import wizard, this 'hands off' mode would amount to do nothing if you elected to not run the tasks. It deserves more thought. I will keep in mind your type of use case, which is import projects 'as is'. However, I imagine even so, there will still be some need to update the eclipse config files now and then, either via the STS tooling calling on the tooling API, or running some gradle tasks to modify the project configs. I'm thinking a 'hands off' mode would mostly do nothing unless you, the user, invoked some specific menu option or command to update something explicitly. But I'm starting to see that 'hands-off' versus 'automated' may be a different thing than 'generate only mode' versus 'query the tooling API mode'. The generate option is more about how it refreshes state whereas the 'hands-off' be more about the when (and who or what triggers refreshes). So you are right... that would be a separate concern from STS-2085 .
        Hide
        Kris De Volder (c) added a comment -

        I wonder if you could provide an example of how the order is being changed. I.e. which types of entries are moving around from import to import?

        From your initial discription, I am guessing it is subproject entries, like this example:

        <classpathentry exported="true" kind="src" path="/spring-security-web"/> 
        

        ... that are switching places?

        If so, these entries are added in precisely the order that the Gradle tooling API returns them. Maybe that order is actually not predictable.

        I'll work on something that takes more direct control of the order entries.
        In the mean time, I'd appreciate some examples if you can manage it (to make sure I'm actually solving the right problem).

        Show
        Kris De Volder (c) added a comment - I wonder if you could provide an example of how the order is being changed. I.e. which types of entries are moving around from import to import? From your initial discription, I am guessing it is subproject entries, like this example: <classpathentry exported="true" kind="src" path="/spring-security-web"/> ... that are switching places? If so, these entries are added in precisely the order that the Gradle tooling API returns them. Maybe that order is actually not predictable. I'll work on something that takes more direct control of the order entries. In the mean time, I'd appreciate some examples if you can manage it (to make sure I'm actually solving the right problem).
        Hide
        Kris De Volder (c) added a comment -

        I've committed a fix. All code setting JDT classpath have been refactored and pulled into one place. A class that explicitly maintains classpath entries in a well-defined, sorted order:

        • source folders first
        • then project entries
        • then library entries
        • then classpath container entries
        • then classpath variable entries (but there shouldn't be any)

        Within each category the entries are sorted alphabetically based on the 'path' of the entry.

        I did it this way because it seems we can't depend on the entries returned by Gradle itself to be in a stable ordering.

        I've also added some extra checks to avoid setting the classpath if it didn't have any changes.

        Show
        Kris De Volder (c) added a comment - I've committed a fix. All code setting JDT classpath have been refactored and pulled into one place. A class that explicitly maintains classpath entries in a well-defined, sorted order: source folders first then project entries then library entries then classpath container entries then classpath variable entries (but there shouldn't be any) Within each category the entries are sorted alphabetically based on the 'path' of the entry. I did it this way because it seems we can't depend on the entries returned by Gradle itself to be in a stable ordering. I've also added some extra checks to avoid setting the classpath if it didn't have any changes.
        Hide
        Kris De Volder (c) added a comment -

        Since I don't think the fix for this issue really addresses all of your concern re the 'as is' importing...
        and since I agree this isn't quite the same as STS-2085, I've raised this issue:

        https://issuetracker.springsource.com/browse/STS-2179

        Show
        Kris De Volder (c) added a comment - Since I don't think the fix for this issue really addresses all of your concern re the 'as is' importing... and since I agree this isn't quite the same as STS-2085 , I've raised this issue: https://issuetracker.springsource.com/browse/STS-2179
        Hide
        Mauro Molinari added a comment - - edited

        Hi Kris,
        thanks for your support.
        I'll comment on STS-2179.

        Regarding this issue, yes, the sorting problem was on the subproject entries.

        Your fix will surely help, but I think that when you say "we can't depend on the entries returned by Gradle itself to be in a stable ordering" you're outlining a problem.

        I mean, I'm not a Gradle expert (I'm learning now), so I don't know if the compile dependencies declared in Gradle build script will actually create an ordered classpath, but I know for sure that the concept of "order" in the classpath does exist. Eclipse, for instance, shows the order of the referenced libraries/projects in the Order and Export tab of the Java Build Path property page of a project. So, if in my Gradle build script I declare that project A depends (for compiling) on B and C (in this order), I think this should be mapped to the fact that A classpath includes B and C ones in this exact order, so that if a class package.MyClass is both in B an C classpaths, that of B should be picked up by the compiler.
        If Gradle can handle this, I think that the API it exposes to STS should be coherent and expose the dependencies in the declared (and so predictable) order. In this way, STS could map the dependencies in a predictable way and change the .classpath file of the A project accordingly, without the need to apply a different sorting algorithm.

        What do you think? Should I open an issue in Gradle JIRA?

        Show
        Mauro Molinari added a comment - - edited Hi Kris, thanks for your support. I'll comment on STS-2179 . Regarding this issue, yes, the sorting problem was on the subproject entries. Your fix will surely help, but I think that when you say "we can't depend on the entries returned by Gradle itself to be in a stable ordering" you're outlining a problem. I mean, I'm not a Gradle expert (I'm learning now), so I don't know if the compile dependencies declared in Gradle build script will actually create an ordered classpath, but I know for sure that the concept of "order" in the classpath does exist. Eclipse, for instance, shows the order of the referenced libraries/projects in the Order and Export tab of the Java Build Path property page of a project. So, if in my Gradle build script I declare that project A depends (for compiling) on B and C (in this order), I think this should be mapped to the fact that A classpath includes B and C ones in this exact order, so that if a class package.MyClass is both in B an C classpaths, that of B should be picked up by the compiler. If Gradle can handle this, I think that the API it exposes to STS should be coherent and expose the dependencies in the declared (and so predictable) order. In this way, STS could map the dependencies in a predictable way and change the .classpath file of the A project accordingly, without the need to apply a different sorting algorithm. What do you think? Should I open an issue in Gradle JIRA?
        Hide
        Kris De Volder (c) added a comment -

        I agree with you that the order really should be stable (and in certain cases, when there conflicts in classloading, order may even make a difference in how classes resolve).

        Since you tell me you have seen the order of those project dependencies changing on successive imports, I just concluded they were not stable as I'm adding them in the order I get them from Gradle, using a type of collection (LinkedHashSet) that preserves order.

        However, I haven't actually observed a changing order case. I.e. during my testing I didn't see this happening. So before raising a bug with Gradle about this, we probably really need some more evidence (i.e. a demonstrative example).

        Before seeing an actual example, we have to consider it is also possible that I've overlooked something and my code was to blame for the reordering.

        In any case, I think the changes I made are probably good, since now my code is taking a much more active role in putting entries in a logical and stable ordering. It is also easier now to change that code and change the ordering.

        Show
        Kris De Volder (c) added a comment - I agree with you that the order really should be stable (and in certain cases, when there conflicts in classloading, order may even make a difference in how classes resolve). Since you tell me you have seen the order of those project dependencies changing on successive imports, I just concluded they were not stable as I'm adding them in the order I get them from Gradle, using a type of collection (LinkedHashSet) that preserves order. However, I haven't actually observed a changing order case. I.e. during my testing I didn't see this happening. So before raising a bug with Gradle about this, we probably really need some more evidence (i.e. a demonstrative example). Before seeing an actual example, we have to consider it is also possible that I've overlooked something and my code was to blame for the reordering. In any case, I think the changes I made are probably good, since now my code is taking a much more active role in putting entries in a logical and stable ordering. It is also easier now to change that code and change the ordering.
        Hide
        Mauro Molinari added a comment - - edited

        I observed the changing order while working with my co-worker. He works in Linux and I work in Windows and maybe this problem happens also because we're using the following code in settings.gradle of the main project to include all the subprojects of the main project:

        settings.gradle
        rootDir.traverse(
        	type: groovy.io.FileType.FILES,
        	nameFilter: ~/.+\.gradle/,
        	maxDepth: 3,
        	preDir: { path << it.name },
        	postDir: { path.removeLast() }) { if (path) include path.join(":") }
        	 
        

        That means: pick up all the projects in the subfolders you find having any *.gradle file.
        Maybe the way folders are iterated (which might be different in my system compared to my co-worker's one) are influencing the ordering of the dependencies calculated by Gradle?

        Anyway, I'm going to make some further tests before and after the new nightly build is available.

        Show
        Mauro Molinari added a comment - - edited I observed the changing order while working with my co-worker. He works in Linux and I work in Windows and maybe this problem happens also because we're using the following code in settings.gradle of the main project to include all the subprojects of the main project: settings.gradle rootDir.traverse( type: groovy.io.FileType.FILES, nameFilter: ~/.+\.gradle/, maxDepth: 3, preDir: { path << it.name }, postDir: { path.removeLast() }) { if (path) include path.join( ":" ) } That means: pick up all the projects in the subfolders you find having any *.gradle file. Maybe the way folders are iterated (which might be different in my system compared to my co-worker's one) are influencing the ordering of the dependencies calculated by Gradle? Anyway, I'm going to make some further tests before and after the new nightly build is available.
        Hide
        Kris De Volder (c) added a comment -

        Yes, if gradle returns the dependencies in the order that the build script produces them (I don't know this for sure, but I assume this would be the case) and you have code like this, I would expect the order to vary based on OS.

        In fact, I've seen this kind of behavior elsewhere (grails dependencies) and the reason was that os operations for listing files in directories return the files in different orders based on the file system / os.

        In essence, this makes the ordering your (writer of the build script) responsibility.

        I think it makes sense to sort the entries in STS though, but perhaps if someone really explicitly and consciously wrote their build script to control the order precisely, they would need some option to disable that sorting as well.

        Show
        Kris De Volder (c) added a comment - Yes, if gradle returns the dependencies in the order that the build script produces them (I don't know this for sure, but I assume this would be the case) and you have code like this, I would expect the order to vary based on OS. In fact, I've seen this kind of behavior elsewhere (grails dependencies) and the reason was that os operations for listing files in directories return the files in different orders based on the file system / os. In essence, this makes the ordering your (writer of the build script) responsibility. I think it makes sense to sort the entries in STS though, but perhaps if someone really explicitly and consciously wrote their build script to control the order precisely, they would need some option to disable that sorting as well.
        Hide
        Mauro Molinari added a comment -

        Hi Kris,
        after some deep investigation I found out that it was our fault. We were creating project dependencies by iterating over collections which do not guarantee the iteration order.

        So, it seems like STS-Gradle integration did respect the order of dependencies calculated by Gradle and that this one is giving dependencies in the correct order.

        This said, my report was invalid and from my personal point of view it is better if the Gradle Import Wizard respects this ordering rather than applying its own sorting criteria. So, personally speaking, I would revert the changes introduced here, but I'd like to know what you think about it.

        Sorry for the waste of time Actually, I'm still investigating on this issue because I'm trying to explain myself some other strange behaviours I see in our main (huge) project. I will report back more precisely as soon as possible.

        Show
        Mauro Molinari added a comment - Hi Kris, after some deep investigation I found out that it was our fault. We were creating project dependencies by iterating over collections which do not guarantee the iteration order. So, it seems like STS-Gradle integration did respect the order of dependencies calculated by Gradle and that this one is giving dependencies in the correct order. This said, my report was invalid and from my personal point of view it is better if the Gradle Import Wizard respects this ordering rather than applying its own sorting criteria. So, personally speaking, I would revert the changes introduced here, but I'd like to know what you think about it. Sorry for the waste of time Actually, I'm still investigating on this issue because I'm trying to explain myself some other strange behaviours I see in our main (huge) project. I will report back more precisely as soon as possible.
        Hide
        Kris De Volder (c) added a comment -

        Well, it is not entirely a waste of time. I did find that my code was a bit sloppy with ordering...

        Entries returned by Gradle come in classes "source folder", "project deps" and "external". (No order is really suggested or defined between the entries, but the entries in each category I guess, do get returned in some order).

        My code wasn't all that careful what order the categories come in though it did respect order within the category.

        Now all the code dealing with this is centralized and follows a 'principle'. The principle can be changed by changing the code, so all that is really a good thing.

        I could 'revert' to respecting the order returned of project deps returned by gradle, but keep the improved code structure. Or I could make it a user controllable preference whether the order should be 'sorted' or 'as defined by build script'.

        I don't fault you at all. I imagine it may happen to other people in the same/similar ways... so it may in fact be a good idea to sort things by default, since my hunch is that most people will not write their build script being conscious of the ordering. In fact, mostly they won't even care, until they find that SVN or CVS is continually flagging their .classpath files as changed.

        So my proposal would be:

        • make sorting the default behavior
        • allow 'disabling' sorting with a preference that will be stored in the .gradle-sts-settings of the root project.

        What do you think about this?

        Show
        Kris De Volder (c) added a comment - Well, it is not entirely a waste of time. I did find that my code was a bit sloppy with ordering... Entries returned by Gradle come in classes "source folder", "project deps" and "external". (No order is really suggested or defined between the entries, but the entries in each category I guess, do get returned in some order). My code wasn't all that careful what order the categories come in though it did respect order within the category. Now all the code dealing with this is centralized and follows a 'principle'. The principle can be changed by changing the code, so all that is really a good thing. I could 'revert' to respecting the order returned of project deps returned by gradle, but keep the improved code structure. Or I could make it a user controllable preference whether the order should be 'sorted' or 'as defined by build script'. I don't fault you at all. I imagine it may happen to other people in the same/similar ways... so it may in fact be a good idea to sort things by default, since my hunch is that most people will not write their build script being conscious of the ordering. In fact, mostly they won't even care, until they find that SVN or CVS is continually flagging their .classpath files as changed. So my proposal would be: make sorting the default behavior allow 'disabling' sorting with a preference that will be stored in the .gradle-sts-settings of the root project. What do you think about this?
        Hide
        Mauro Molinari added a comment -

        I'm glad to know it was not a total waste of time.
        I like the idea of making Gradle integration configurable about sorting. By default, however, I would leave the sorting criteria "as defined by build script". In this way, the new option would be backward compatible and I think most people (at least the ones who care about this) would expect that by default the Gradle import wizard does respect what is written in the build script.

        Show
        Mauro Molinari added a comment - I'm glad to know it was not a total waste of time. I like the idea of making Gradle integration configurable about sorting. By default, however, I would leave the sorting criteria "as defined by build script". In this way, the new option would be backward compatible and I think most people (at least the ones who care about this) would expect that by default the Gradle import wizard does respect what is written in the build script.
        Hide
        Kris De Volder (c) added a comment -

        I'm just thinking, which default setting will work to make life easier for the largest percentage of user?

        My guess would be that largest number of users wouldn't be conscious or care about the order when writing the build script. And these users would be best served by having the entries sorted by the tooling.

        Anyhow, I tend to defer to the opinion of users on choices like this. So since you are the only user who I have an opinion for I'd probably follow your advice here. Though it would be reasuring to have more than one user's opion

        Anyhow, I'll leave it for a couple of days before implementing this.

        I'll reopen this issue now to remember there's something left to do.

        Show
        Kris De Volder (c) added a comment - I'm just thinking, which default setting will work to make life easier for the largest percentage of user? My guess would be that largest number of users wouldn't be conscious or care about the order when writing the build script. And these users would be best served by having the entries sorted by the tooling. Anyhow, I tend to defer to the opinion of users on choices like this. So since you are the only user who I have an opinion for I'd probably follow your advice here. Though it would be reasuring to have more than one user's opion Anyhow, I'll leave it for a couple of days before implementing this. I'll reopen this issue now to remember there's something left to do.
        Hide
        Kris De Volder (c) added a comment -

        TODO: make sorting / not sorting user configurable

        Show
        Kris De Volder (c) added a comment - TODO: make sorting / not sorting user configurable
        Hide
        Davide Cavestro added a comment -

        My guess would be that largest number of users wouldn't be conscious or care about the order when writing the build script. And these users would be best served by having the entries sorted by the tooling.

        PREMISE: I'm a co-worker of Mauro's (in fact we avoid voting each other bugs) so don't take my opinion for statistic purposes.
        That said, maybe it could be a reasonable choice implementing the entries sorted by the tooling as a default/first choice way, provided that the IDE remembers last user choice, so when the developer choose to preserve the gradle script original sorting, then it becomes the default choice for that environment.
        Just my two cents
        thank you for your promptness

        Cheers
        Davide

        Show
        Davide Cavestro added a comment - My guess would be that largest number of users wouldn't be conscious or care about the order when writing the build script. And these users would be best served by having the entries sorted by the tooling. PREMISE: I'm a co-worker of Mauro's (in fact we avoid voting each other bugs) so don't take my opinion for statistic purposes. That said, maybe it could be a reasonable choice implementing the entries sorted by the tooling as a default/first choice way, provided that the IDE remembers last user choice, so when the developer choose to preserve the gradle script original sorting, then it becomes the default choice for that environment. Just my two cents thank you for your promptness Cheers Davide
        Hide
        Kris De Volder (c) added a comment -

        Yes, I would imagine that we store sorting preference in the root project's settings. So the settings could be stored in SVN/CVS and shared between users... and will all apply to all projects in a given build.

        Show
        Kris De Volder (c) added a comment - Yes, I would imagine that we store sorting preference in the root project's settings. So the settings could be stored in SVN/CVS and shared between users... and will all apply to all projects in a given build.
        Hide
        Kris De Volder (c) added a comment -

        Committed a fix with regression test.

        There is now a project property page accesible by right click on a Gradle project. This allows setting whether entries should be sorted or maintain the order as returned by the build script.

        This preference is stored in the root project .settings so it is shared for all projects in the same hierarchy.

        Show
        Kris De Volder (c) added a comment - Committed a fix with regression test. There is now a project property page accesible by right click on a Gradle project. This allows setting whether entries should be sorted or maintain the order as returned by the build script. This preference is stored in the root project .settings so it is shared for all projects in the same hierarchy.

          People

          • Assignee:
            Kris De Volder (c)
            Reporter:
            Mauro Molinari
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:
              First Response Date: