Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding 'fast' profile for building Che faster by skipping unit tests,… #2733

Merged
merged 1 commit into from
Oct 7, 2016

Conversation

ibuziuk
Copy link
Member

@ibuziuk ibuziuk commented Oct 7, 2016

What does this PR do?

'fast' profile for building Che faster by skipping unit tests, license checks and other enforcement features

What issues does this PR fix or reference?

#2594

New behavior

"mvn clean verify -Pfast" will allow to build che with the following system properties enabled:

    -DskipTests=true
    -Dfindbugs.skip=true
    -Dskip-validate-sources
    -Dmdep.analyze.skip=true
    -Dlicense.skip=true
    -Dgwt.compiler.localWorkers=2 -T 1C

PR type

  • Minor change = no change to existing features or docs
  • Major change = changes existing features or docs

Minor change checklist

  • New API required?
  • API updated
  • Tests provided / updated
  • Tests passed

Major change checklist

  • New API required?
  • API updated
  • Documentation provided (include here or link to docs)
  • Tests provided / updated
  • Tests passed

Signed-off-by: Ilya Buziuk ibuziuk@redhat.com

… license checks and other enforcement features

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
@codenvy-ci
Copy link

Can one of the admins verify this patch?

@benoitf
Copy link
Contributor

benoitf commented Oct 7, 2016

ci-build

@TylerJewell TylerJewell merged commit 28f5184 into eclipse-che:master Oct 7, 2016
@TylerJewell TylerJewell added this to the 5.0.0-M6 milestone Oct 7, 2016
@TylerJewell TylerJewell added the kind/enhancement A feature request - must adhere to the feature request template. label Oct 7, 2016
@benoitf
Copy link
Contributor

benoitf commented Oct 11, 2016

@ibuziuk After some tests of this profile it looks like -Pfast is not disabling the validate source profile

using -Dskip-validate-sources is working but using -Pfast not.

$ mvn -Pfast help:active-profiles |grep validate-sources
 - validate-sources (source: org.eclipse.che.parent:maven-parent-pom:5.0.0-M6-SNAPSHOT)
 - validate-sources (source: org.eclipse.che.parent:maven-parent-pom:5.0.0-M6-SNAPSHOT)
 - validate-sources (source: org.eclipse.che.parent:maven-parent-pom:5.0.0-M6-SNAPSHOT)
...
$ mvn -Pfast -Dskip-validate-sources help:active-profiles |grep validate-sources

@ibuziuk
Copy link
Member Author

ibuziuk commented Oct 11, 2016

@benoitf oh, this is trickier than I expected it to be. Basically, one can not activate a profile when another one is activated (chaining activation is prohibited in maven). skip-validate-sources actually enables another profile - https://github.com/eclipse/che-parent/blob/master/pom.xml#L1651 . So, the only solution for this situation would be activating both profiles via the same property e.g.

<activation>
  <property>
    <name>fast</name>
  </property>
</activation>

I guess, if we change the fast profile's activation^ and add fast property to validate-source profile activation both should be enabled via mvn clean verify -Dfast

@benoitf
Copy link
Contributor

benoitf commented Oct 11, 2016

@ibuziuk note that we also have skip-enforce property.
also I'm not sure it's about chained profile but just that property is read and enable the provile before another profile set value to false
I'm then not sure to have fast to disable their activation (because it's disabled profile if property is set)

@ibuziuk
Copy link
Member Author

ibuziuk commented Oct 11, 2016

@benoitf I do believe if we may have smth. like that:

      <profile>
            <id>validate-sources</id>
            <activation>
                <property>
                    <name>!skip-validate-sources</name>
                    <name>fast</name>
                </property>
            </activation>

AFAIK, This will not disable validate-sources profile but rather add one more condition when it should be enabled. Maven<activation> block is a list of 'or' statements and profile will be activated as soon as the first criteria is met

@ibuziuk
Copy link
Member Author

ibuziuk commented Oct 11, 2016

@benoitf sorry I was wrong about 'or' statements, it used to be like that before v3.2.2 but now it is fixed and it is actually the list of 'and' https://issues.apache.org/jira/browse/MNG-4565

@benoitf
Copy link
Contributor

benoitf commented Oct 11, 2016

OK

@TylerJewell
Copy link

Cool.

@ibuziuk
Copy link
Member Author

ibuziuk commented Oct 11, 2016

Trying to figure out how it is possible to configure 'or' logic. It seems (and I would really like to be wrong here) that that after the "fix" it is not possible to setup "OR" logic for profile activation :/

@ibuziuk
Copy link
Member Author

ibuziuk commented Oct 14, 2016

@benoitf it is not possible to configure IF condition for maven profile activation unless using a copy-paste approach - having two identical profiles with different activation logic. At the same time, chaining profile activation is also prohibited. So, there are two possible things we can do:

  1. Remove skip-validate-source from fast profile (using the profile would include resource validation step )
  2. duplicate skip-validate-source profile with different activation logic e.g.
<activation>
  <property>
    <name>fast</name>
  </property>
</activation>

and change fast profile activation logic the same^ way

WDYT ?

JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
… license checks and other enforcement features (eclipse-che#2733)

Signed-off-by: Ilya Buziuk <ibuziuk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants