Reuse checkstyle configuration

[Ignasi <ig...@gmail.com>]

Hi!

I'm working on JCLOUDS-149 [1] to make the checkstyle configuration
available to all repos. There are three options:

1. Copy the configuration to all repos (not maintainable).
2. Use a public URL pointing to the ckeckstyle.xml (offline builds
won't be able to run checkstyle).
3. Add the checkstyle.xml to the class path and reference it in the
maven-checkstyle-plugin configuration.

I've made some changes in the jclouds-resources [2] project to include
the checkstyle.xml in the generated jar, so it can be read from all
repos. This configuration works for the other repos (I've tried it
with jclouds-chef), but not entirely for the main repo. I'm quite
blocked here, so let's summon the Maven gurus:

In order to use the checkstyle.ml config from the classpath, the
maven-checkstyle-plugin must declare the jclouds-resources dependency
in its configuration. The problem is that the plugin is configured in
the jclouds-project, which is the parent pom, and that means the
jclouds-project needs to have the jclouds-resources.jar in the maven
repo *before* it can be actually built, and this is where the build
breaks.

I haven't figured out how to properly configure this (if possible).
Another option could be to reference file in the main repo as a local
file, and override the maven-checkstyle-plugin configuration in every
repo to use the one in the class path. That should work *but* that
means overriding the plugin config in every pom.xml that inherits from
jclouds-project (and this is every single maven project in labs, which
is not practical).


Does anyone have a better approach or ideas?


Thanks!


[1] https://issues.apache.org/jira/browse/JCLOUDS-149
[2] https://github.com/nacx/jclouds/commit/02199355b959bedea87f88f138503702ce549ada

Re: Reuse checkstyle configuration

[Andrew Phillips <ap...@qrmedia.com>]

Yay!

Thanks, Ignasi!

ap

Re: Reuse checkstyle configuration

[Ignasi <ig...@gmail.com>]

Finally this has been merged!

I've re-enabled checkstyle goals in the labs pull request builds.


Thx!

On 29 June 2013 22:11, Ignasi <ig...@gmail.com> wrote:
> I also like it, so let's go for option 2 then :)
>
> Many thanks for spending your time on this!
>
> I.
>
> El 29/06/2013 21:58, "Andrew Phillips" <ap...@qrmedia.com> escribió:
>
>> TL;DR: I'm for option 2 ;-)
>>
>>> This second approach has one limitation: The jclouds-resources.jar
>>> must be in the local maven repository, in order to let the modules get
>>> the checkstyle configuration. This means that the following command
>>> may not work because the jclouds-resources.jar will only be built but
>>> not installed to the local Maven repo.
>>
>>
>> Wouldn't you be able to get this dependency from the Apache snapshots repo
>> (or Maven Central if you're trying to build a released version)?
>>
>> Getting hold of a snapshot dependency without first installing it yourself
>> is a problem all the labs repos have with jclouds-project (their new parent)
>> anyway.
>>
>> If we feel it's acceptable for them to get their *parent* dep from a
>> published (rather than the local) repo, I think we should be OK with that
>> approach for checkstyle too.
>>
>> It's true that the leaves a small time window in which changes to the
>> checkstyle will not be applied to labs PR builds (if the PR build triggers
>> before the job for the main project updates the snapshots repo), but I think
>> we can live with that?
>>
>> Hopefully, changes to the checkstyle config will be infrequent enough that
>> this won't be a big problem in practice. And in the worst case, we can
>> always re-trigger PR builds manually if they ran "too early" and failed
>> because they missed a checkstyle update.
>>
>> ap

Re: Reuse checkstyle configuration

[Ignasi <ig...@gmail.com>]

I also like it, so let's go for option 2 then :)

Many thanks for spending your time on this!

I.
El 29/06/2013 21:58, "Andrew Phillips" <ap...@qrmedia.com> escribió:

> TL;DR: I'm for option 2 ;-)
>
>  This second approach has one limitation: The jclouds-resources.jar
>> must be in the local maven repository, in order to let the modules get
>> the checkstyle configuration. This means that the following command
>> may not work because the jclouds-resources.jar will only be built but
>> not installed to the local Maven repo.
>>
>
> Wouldn't you be able to get this dependency from the Apache snapshots repo
> (or Maven Central if you're trying to build a released version)?
>
> Getting hold of a snapshot dependency without first installing it yourself
> is a problem all the labs repos have with jclouds-project (their new
> parent) anyway.
>
> If we feel it's acceptable for them to get their *parent* dep from a
> published (rather than the local) repo, I think we should be OK with that
> approach for checkstyle too.
>
> It's true that the leaves a small time window in which changes to the
> checkstyle will not be applied to labs PR builds (if the PR build triggers
> before the job for the main project updates the snapshots repo), but I
> think we can live with that?
>
> Hopefully, changes to the checkstyle config will be infrequent enough that
> this won't be a big problem in practice. And in the worst case, we can
> always re-trigger PR builds manually if they ran "too early" and failed
> because they missed a checkstyle update.
>
> ap
>

Re: Reuse checkstyle configuration

[Andrew Phillips <ap...@qrmedia.com>]

TL;DR: I'm for option 2 ;-)

> This second approach has one limitation: The jclouds-resources.jar
> must be in the local maven repository, in order to let the modules get
> the checkstyle configuration. This means that the following command
> may not work because the jclouds-resources.jar will only be built but
> not installed to the local Maven repo.

Wouldn't you be able to get this dependency from the Apache snapshots  
repo (or Maven Central if you're trying to build a released version)?

Getting hold of a snapshot dependency without first installing it  
yourself is a problem all the labs repos have with jclouds-project  
(their new parent) anyway.

If we feel it's acceptable for them to get their *parent* dep from a  
published (rather than the local) repo, I think we should be OK with  
that approach for checkstyle too.

It's true that the leaves a small time window in which changes to the  
checkstyle will not be applied to labs PR builds (if the PR build  
triggers before the job for the main project updates the snapshots  
repo), but I think we can live with that?

Hopefully, changes to the checkstyle config will be infrequent enough  
that this won't be a big problem in practice. And in the worst case,  
we can always re-trigger PR builds manually if they ran "too early"  
and failed because they missed a checkstyle update.

ap

Re: Reuse checkstyle configuration

[Ignasi <ig...@gmail.com>]

Yes, that makes sense, but I think I didn't explain properly the problem :)

What you say is the 2nd option I suggested: one default profile that works
for every project in any repo, and a custom profile for jclouds-project to
avoid the dependency issue (but it presents the mvn clean verify
limitation).

To avoid the "mvn clean verify" issue, the default profile should not pick
the checkstyle config from the classpath. This is the current config in
master (but it does not work for other repos as they don't have a copy of
the checkstyle config file). The ideal thing should be to keep this default
profile and have another one that is only active for the other repos, but I
haven't found how to configure it, due to the limitations of the profile
activation I mentioned.

And I'm stuck there.

Does this make sense now, or am I just complicating too much a simple thing
I can't figure out?

Thanks for the follow up!
El 29/06/2013 16:03, "Andrew Phillips" <ap...@qrmedia.com> escribió:

> The example is great, but it only activates the profile for the
>> jclouds-project module, and we'll need it for every module in the
>> jclouds repo. I've added a new profile and tried to configure its
>> activation to be enabled for all projects, but:
>>
>
> Wait...the idea behind the profile is to *deactivate* remote-resources! So
> it's indeed intentional that the profile is *only* active in
> jclouds-project.
>
> There's a *default* invocation (not in a profile) that runs for *all*
> projects. The profile is only intended to override that default invocation
> to stop remote-resources being run in jclouds-project, which causes the
> missing dependency problem you saw.
>
> Does this make more sense?
>
> ap
>

Re: Reuse checkstyle configuration

[Andrew Phillips <ap...@qrmedia.com>]

> The example is great, but it only activates the profile for the
> jclouds-project module, and we'll need it for every module in the
> jclouds repo. I've added a new profile and tried to configure its
> activation to be enabled for all projects, but:

Wait...the idea behind the profile is to *deactivate*  
remote-resources! So it's indeed intentional that the profile is  
*only* active in jclouds-project.

There's a *default* invocation (not in a profile) that runs for *all*  
projects. The profile is only intended to override that default  
invocation to stop remote-resources being run in jclouds-project,  
which causes the missing dependency problem you saw.

Does this make more sense?

ap

Re: Reuse checkstyle configuration

[Ignasi <ig...@gmail.com>]

Many thanks for the tip Andrew, it's been very useful!

The example is great, but it only activates the profile for the
jclouds-project module, and we'll need it for every module in the
jclouds repo. I've added a new profile and tried to configure its
activation to be enabled for all projects, but:

* Only file activation can be used, since other types (property based,
etc) would activate the profile also for the other git repositories
that inherit from jclouds-project.
* File based activation only performs variable substitution for system
properties (provided in the command line), but not for properties
generated in the pom.xml, which makes it not flexible enough for our
needs.

These points, and the fact that the modules in the jclouds repo don't
have all the same depth, makes it impossible to configure the profile
with a file activation that works for every Maven module in the
jclouds repo (and not for the others). At least I haven't found how.


Then I can only think about two approaches:

1. By default, look for the configuration in the local path, and have
a separate profile to use the one in the classpath. The labs and other
repos should manually provide the -Pcheckstyle parameter in order to
be able to read the configuration (this can be easily added to the
Jenkins builds) when running the "checkstyle:checkstyle" goal.

2. By default, look for the configuration in the classpath, and have a
profile in the jclouds-project, only activated for it, that uses the
local filesystem one. This way:
* The jclouds-project will still be the first project to be built (no
missing dependency issues).
* The jclouds-resources will be the second project to be built, which
will install the checkstyle config in the local maven repo.
* The other modules in the repo and other repos can be built reading
the checkstyle configuration from the classpath (from the installed
jclouds-resources jar).

This second approach has one limitation: The jclouds-resources.jar
must be in the local maven repository, in order to let the modules get
the checkstyle configuration. This means that the following command
may not work because the jclouds-resources.jar will only be built but
not installed to the local Maven repo.

mvn clean verify checkstyle:checkstyle

Currently this is how the pull requests builds are configured (which
is good; we don'w want the artifacts from PR builds, installed in the
local repo). If we configure the profiles like point 2, the PR builds
would take the checkstyle config from the last deployed snapshot, and
this is not a consistent behavior.

So... Does anyone have a better idea? If not, do we go for approach 1
or approach 2?


Thanks!

Ignasi

On 28 June 2013 15:02, Andrew Phillips <ap...@qrmedia.com> wrote:
>> In order to use the checkstyle.ml config from the classpath, the
>> maven-checkstyle-plugin must declare the jclouds-resources dependency
>> in its configuration. The problem is that the plugin is configured in
>> the jclouds-project, which is the parent pom, and that means the
>> jclouds-project needs to have the jclouds-resources.jar in the maven
>> repo *before* it can be actually built, and this is where the build
>> breaks.
>
>
> What we did when we still used a remote resource for the license files,
> which caused the same problem, is to have a jclouds-project profile [1]
> which *disabled* the remote-resources execution for jclouds-project only.
>
> Checkstyle could still work for jclouds-project, it would simply have to
> refer to the files by path (since they're in the same repo) rather than use
> a dependency.
>
> Let me know if I can help any more with this!
>
> ap
>
> [1]
> https://github.com/jclouds/jclouds/blob/ddfb8e58a5a729cabb5edf954a113ee8127cd7cb/project/pom.xml#L912

Re: Reuse checkstyle configuration

[Andrew Phillips <ap...@qrmedia.com>]

> In order to use the checkstyle.ml config from the classpath, the
> maven-checkstyle-plugin must declare the jclouds-resources dependency
> in its configuration. The problem is that the plugin is configured in
> the jclouds-project, which is the parent pom, and that means the
> jclouds-project needs to have the jclouds-resources.jar in the maven
> repo *before* it can be actually built, and this is where the build
> breaks.

What we did when we still used a remote resource for the license  
files, which caused the same problem, is to have a jclouds-project  
profile [1] which *disabled* the remote-resources execution for  
jclouds-project only.

Checkstyle could still work for jclouds-project, it would simply have  
to refer to the files by path (since they're in the same repo) rather  
than use a dependency.

Let me know if I can help any more with this!

ap

[1]  
https://github.com/jclouds/jclouds/blob/ddfb8e58a5a729cabb5edf954a113ee8127cd7cb/project/pom.xml#L912