Monday, May 15, 2017, 11:40:01 AM, Taher Alkhateeb wrote:
> 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 :)
We don't do anything heavy in configuration phase (I hope).
>> 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.
Well, the error message tells them how to proceed, but otherwise it's
a valid point that one can't even run "gradlew tasks", which is not
logical. So I think I will move the *validaiton* to the doFirst of
compilation tasks.
> 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?
Solving any of the TODO-s is welcome (preferably with pull requests).
It's not that I'm stuck with them, I just haven't even started dealing
with them.
> 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
>>
>>
--
Thanks,
Daniel Dekany
