grails / grails-wrapper

Grails Wrapper
Apache License 2.0
3 stars 6 forks source link

grailsWrapperVersion is added to gradle.properties (3.3.8) #4

Open dlehammer opened 5 years ago

dlehammer commented 5 years ago

Symptom discovered during migration of web-application from Grails v2.4.4 to v3.3.5, symptom still present in Grails v3.3.8.

Task List

Steps to Reproduce

  1. create a Grails app (symptom is also present for plugins etc.)

    $ grails create-app my-app
  2. commit create-app to version control, ex. using Git.

    my-app$ git init
    my-app$ git add .
    my-app$ git commit --message="grails create-app my-app"
  3. execute a Gradle/Grails command, ex. -version where side-effects are unexpected (symptom is also present for create-service etc.)

    my-app$ ./grailsw -version
    | Grails Version: 3.3.8
    | Groovy Version: 2.4.7
    | JVM Version: 1.8.0_172
  4. now gradle.properties has been modified.

    my-app$ git status 
    On branch master
    Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
    (use "git checkout -- <file>..." to discard changes in working directory)
    
    modified:   gradle.properties
my-app$ git diff gradle.properties
diff --git a/gradle.properties b/gradle.properties
index df75075..c19fdd5 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -1,3 +1,5 @@
+#Sat Dec 01 14:08:20 CET 2018
 grailsVersion=3.3.8
+grailsWrapperVersion=1.0.0
 gormVersion=6.1.10.RELEASE
 gradleWrapperVersion=3.5

Expected Behaviour

gradle.properties isn't expected to be modified by Gradle/Grails after initial create-app.

The Grails wrapper is expected to be self-contained, ie. it's unexpected that redundant configuration leaks into gradle.properties. In my experience the grailsWrapperVersion isn't needed in-order for Gradle/Grails wrapper to function as expected.

Actual Behaviour

gradle.properties is modified by Gradle/Grails, se also Steps to Reproduce

Work-around(s)

A. explicitly add the grailsWrapperVersion=1.0.0 to gradle.properties and commit.

B. mark gradle.properties as readonly.

Environment Information

jeffscottbrown commented 5 years ago

The code at https://github.com/grails/grails-core/blob/6d2949b3aa120da9d6c568c6cc4665eaa819d9b3/grails-shell/src/main/groovy/org/grails/cli/gradle/GradleUtil.groovy#L55 is relevant.

dlehammer commented 5 years ago

@jeffbrown

I sorry, but I must be missing something, the referenced GradleUtil.groovy doesn't refer to grailsWrapperVersion. (however it does reference gradleWrapperVersion :) in line 59)

jeffscottbrown commented 5 years ago

You are correct. My mistake. I didn't read closely enough. I am sorry for the noise.

jameskleeh commented 5 years ago

This is by design. It was put there in order to determine if there is an updated version of the wrapper. In practice that feature was never used and turned out to be probably not the best idea.