fix changed files helper to also get untracked files

Author: rjernst

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy

@@ -165,6 +165,25 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
         ]
     }
 
+    def "unreferenced definitions should be deleted"() {
+        given:
+        namedTransportVersion("old_name", "8124000")
+        referencedTransportVersion("new_name")
+
+        when:
+        def result = gradleRunner(
+            ":myserver:validateTransportVersionDefinitions",
+            ":myserver:generateTransportVersionDefinition",
+            "--name=new_name" ,
+            "--branches=9.2"
+        ).build()
+
+        then:
+        !file("myserver/src/main/resources/transport/definitions/named/old_name.csv").exists()
+        result.task(":myserver:generateTransportVersionDefinition").outcome == TaskOutcome.SUCCESS
+        result.task(":myserver:validateTransportVersionDefinitions").outcome == TaskOutcome.SUCCESS
+    }
+
 
     /*
     TODO: Add tests that check that:

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java

@@ -100,10 +100,10 @@ public GenerateTransportVersionDefinitionTask(ExecOperations execOperations) {
     public void run() throws IOException {
         TransportVersionResourcesService resources = getResources().get();
         Set<String> referencedNames = TransportVersionReference.collectNames(getReferencesFiles());
-        List<TransportVersionDefinition> changedDefinitions = resources.getChangedNamedDefinitions();
+        List<String> changedDefinitionNames = resources.getChangedNamedDefinitionNames();
         String name = getTransportVersionName().isPresent()
             ? getTransportVersionName().get()
-            : findAddedTransportVersionName(resources, referencedNames, changedDefinitions);
+            : findAddedTransportVersionName(resources, referencedNames, changedDefinitionNames);
 
         if (name.isEmpty()) {
             resetAllLatestFiles(resources);
@@ -113,7 +113,7 @@ public void run() throws IOException {
             resources.writeNamedDefinition(new TransportVersionDefinition(name, ids));
         }
 
-        removeUnusedNamedDefinitions(resources, referencedNames, changedDefinitions);
+        removeUnusedNamedDefinitions(resources, referencedNames, changedDefinitionNames);
     }
 
     private List<TransportVersionId> updateLatestFiles(TransportVersionResourcesService resources, String name) throws IOException {
@@ -195,21 +195,21 @@ private void resetAllLatestFiles(TransportVersionResourcesService resources) thr
     private void removeUnusedNamedDefinitions(
         TransportVersionResourcesService resources,
         Set<String> referencedNames,
-        List<TransportVersionDefinition> changedDefinitions
+        List<String> changedDefinitions
     ) throws IOException {
-        for (TransportVersionDefinition definition : changedDefinitions) {
-            if (referencedNames.contains(definition.name()) == false) {
+        for (String definitionName : changedDefinitions) {
+            if (referencedNames.contains(definitionName) == false) {
                 // we added this definition file, but it's now unreferenced, so delete it
-                getLogger().lifecycle("Deleting unreferenced named transport version definition [" + definition.name() + "]");
-                resources.deleteNamedDefinition(definition.name());
+                getLogger().lifecycle("Deleting unreferenced named transport version definition [" + definitionName + "]");
+                resources.deleteNamedDefinition(definitionName);
             }
         }
     }
 
     private String findAddedTransportVersionName(
         TransportVersionResourcesService resources,
         Set<String> referencedNames,
-        List<TransportVersionDefinition> changedDefinitions
+        List<String> changedDefinitions
     ) throws IOException {
         // First check for unreferenced names. We only care about the first one. If there is more than one
         // validation will fail later and the developer will have to remove one. When that happens, generation
@@ -225,7 +225,7 @@ private String findAddedTransportVersionName(
         if (changedDefinitions.isEmpty()) {
             return "";
         } else {
-            return changedDefinitions.getFirst().name();
+            return changedDefinitions.getFirst();
         }
     }
 }

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/TransportVersionResourcesService.java

@@ -123,16 +123,15 @@ TransportVersionDefinition getNamedDefinitionFromMain(String name) {
         return getMainFile(resourcePath, TransportVersionDefinition::fromString);
     }
 
-    List<TransportVersionDefinition> getChangedNamedDefinitions() throws IOException {
-        List<TransportVersionDefinition> changedDefinitions = new ArrayList<>();
+    List<String> getChangedNamedDefinitionNames() {
+        List<String> changedDefinitions = new ArrayList<>();
         String namedPrefix = NAMED_DIR.toString();
         for (String changedPath : getChangedResources()) {
-            if (changedPath.contains(namedPrefix) == false) { // TODO make this more robust
+            if (changedPath.contains(namedPrefix) == false) {
                 continue;
             }
-            // TODO why are we getting the main file here? Shouldn't we just read the changed file directly?
-            TransportVersionDefinition definition = getMainFile(changedPath, TransportVersionDefinition::fromString);
-            changedDefinitions.add(definition);
+            String name = changedPath.substring(namedPrefix.length() + 1 /* skip slash */, changedPath.length() - 4 /* .csv */);
+            changedDefinitions.add(name);
         }
         return changedDefinitions;
     }
@@ -225,11 +224,18 @@ private Set<String> getMainResources() {
     private Set<String> getChangedResources() {
         if (changedResources.get() == null) {
             synchronized (changedResources) {
+                HashSet<String> resources = new HashSet<>();
+
                 // gitCommand("add", "."); // TODO this finds the files that have been added without being committed.
-                String output = gitCommand("diff", "--name-only", "main", ".");
+                String diffOutput = gitCommand("diff", "--name-only", "main", ".");
+                if (diffOutput.strip().isEmpty() == false) {
+                    Collections.addAll(resources, diffOutput.split(System.lineSeparator()));
+                }
 
-                HashSet<String> resources = new HashSet<>();
-                Collections.addAll(resources, output.split(System.lineSeparator()));
+                String untrackedOutput = gitCommand("ls-files", "--others", "--exclude-standard");
+                if (untrackedOutput.strip().isEmpty() == false) {
+                    Collections.addAll(resources, untrackedOutput.split(System.lineSeparator()));
+                }
                 changedResources.set(resources);
             }
         }