Hi Daniel, Inline ...
On Mon, May 15, 2017 at 12:17 PM, Daniel Dekany <[email protected]> wrote: > Monday, May 15, 2017, 8:27:07 AM, Taher Alkhateeb wrote: > > > Hi Daniel, > > > > Great work! a few issues: > > - the gradlew wrapper script mode needs to switch to be executable (unix > > flag) > > - gradle is running a lot of stuff in the configuration phase, before we > > even know what it does. For example, running ./gradlew tasks gives me the > > below error. This means that not only is it expecting a flag, but also > > executing considerable work in the configuration phase. I prefer to > always > > delay work until I need it, which is usually in the execution phase. > > My understanding is that in Gradle you are supposed to do the > configuration tasks up front. If they take considerable time, that's a > problem and may warrant deviating from that, but in our case I doubt > they take any perceivable time. > First forgive me for my ignorance as I did not check all the details of what's happening exactly in the script, and I guess there are no rights and wrongs, just a preference issue. What I suggested is to prepare things when needed is because sometimes not all tasks require the same preparation, and so running the preparation every time you run gradlew might consume some resources, if not then ignore my comment :) > > As of the error message, do you mean that we should do those checks in > [compileJava, compileTestJava]*.doFirst, so that you can run other > tasks that do not need those properties without setting up things as > the README.md dictates? Then I think we gain little with that as in > practice one can't do any real work without compiling something > anyway, and on the same time it makes the script harder to > understand/maintain, because that's not the standard away of doing > things (moving configuration activity part the configuration phase). > Also note that in the root we provide the defaults, and in some other > subprojects we override them, which works now as it used to be done in > Gradle, but with doFirst it will become trickier. > Again I think no rights and wrongs here, just preferences and consequences. For example, in ant you can type "ant -p" to list tasks. The equivalent of that in gradle is "gradlew tasks". However, this task will simply fail because it cannot proceed past the configuration phase, so people who want to know what tasks are available and how to use them might be a bit confused. The same is true if you want to view projects "./gradlew projects" or other such informational tasks. > > > - I tried to tackle the first TODO that pulls configuration from your > > properties file (a small patch, so I put it in this email). Is this what > > you're looking for? > > > > ==== Patch for properties loading ==== > > diff --git a/build.gradle b/build.gradle > > index d47fa9e..5f4e8cb 100644 > > --- a/build.gradle > > +++ b/build.gradle > > @@ -17,11 +17,12 @@ > > * under the License. > > */ > > > > -// TODO: Versions should come form > > src/main/resource/o/a/f/c/version.properties > > -ext.versionCanonical = "3.0.0-nightly-incubating" > > -ext.versionForMaven = "3.0.0-SNAPSHOT" > > -ext.versionForOSGi = "3.0.0.nightly-incubating" > > -ext.versionForMf = "2.97.0" > > +Properties props = new Properties() > > +props.load(file('freemarker-core/src/main/resources/org/ > apache/freemarker/core/version.properties').newDataInputStream()) > > +ext.versioncanonical = props.getProperty('version') > > +ext.versionformaven = props.getProperty('mavenVersion') > > +ext.versionforosgi = props.getProperty('versionForOSGi') > > +ext.versionformf = props.getProperty('versionForMf') > > Yes, something like that. But I haven't looked after how Gradle will > know if those variables depend on that file (or if have to care about > that at all), what will happen if a property is missing, all the > details like that... > Well, I guess the simplest solution if you want to assign-if-null is simply by changing the above code as follows: ext.versioncanonical = props.getProperty('version', 'MyDefaultValueHere') Please tell me if I can / should help with any of that? HTH > > > allprojects { > > group = "org.apache.freemarker" > > ==== end of patch ==== > > > > ==== error message when running ./gradlew tasks ==== > > % Total % Received % Xferd Average Speed Time Time Time > > Current > > Dload Upload Total Spent Left > > Speed > > 0 0 0 0 0 0 0 0 --:--:-- 0:00:01 > > --:--:-- 0 > > 100 90.6M 100 90.6M 0 0 3225k 0 0:00:28 0:00:28 --:--:-- > > 3975k > > ~/.gradle/wrapper/dists/gradle-3.5-all/fc3129dda6113a2b1fa07735831d27c6 > > ~/Desktop/active/development/work/incubator-freemarker > > ~/Desktop/active/development/work/incubator-freemarker > > Starting a Gradle Daemon (subsequent builds will be faster) > > > > FAILURE: Build failed with an exception. > > > > * Where: > > Build file > > '/home/taher/Desktop/active/development/work/incubator- > freemarker/build.gradle' > > line: 49 > > > > * What went wrong: > > The bootClasspathJava7 property must be set. Maybe you have missed this > > step: Copy gradle.properties.sample into gradle.properties, and edit it > to > > describe your environment. Alternatively, pass the properties to gradle > > with -PbootClasspathJava7="...". > > ==== end of error message ==== > > > > On Sun, May 14, 2017 at 1:54 PM, Daniel Dekany <[email protected]> > wrote: > > > >> I have committed a first version of this... It's already modularized > >> as you can see, though freemarker-dom is not yet separated from core. > >> > >> Please find the problems with it. Also, anyone wants to help with the > >> TODO-s in the Gradle scripts? > >> > >> -- > >> Thanks, > >> Daniel Dekany > >> > >> > > -- > Thanks, > Daniel Dekany > >
