Ability to include / exclude files with patterns for javascript compilation / aggregation / minification and same for less #480

Closed
jbrey opened this Issue Apr 22, 2015 · 16 comments

Comments

Projects
None yet
2 participants
@jbrey
Contributor

jbrey commented Apr 22, 2015

Hey,

It would be great if we would have the ability to configure includes and excludes patterns when we configure:

  • javascript compilation, example:
    <excludes>
          <exclude>**/*.map*</exclude>
     </excludes>
  • javascript aggregation, as far as I understand we need to list every files and cannot use patterns like that:
    <includes>
           <include>myfolder/**/*.js</include>
    </includes>
  • javascript minification

and the same for every type of resources.

Is it planned to support these configuration ? does it make sense for you ?

Cheers
JB

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 22, 2015

Member

We don't support wildcard for a reason: it's not deterministic. Aggregation must be deterministic if you don't want to have really weird issue sometimes on some platform because the order is different.

Member

cescoffier commented Apr 22, 2015

We don't support wildcard for a reason: it's not deterministic. Aggregation must be deterministic if you don't want to have really weird issue sometimes on some platform because the order is different.

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 22, 2015

Member

(file order depends on the operating system)

Member

cescoffier commented Apr 22, 2015

(file order depends on the operating system)

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 22, 2015

Contributor

ok for the specific case of aggregation.
We could think of an alternated solution where the list of can compose the order of the aggreation then:

       <includes>
                 <include>src/main/javascript/core/the-first-to-aggregate.js</include>
                 <include>src/main/javascript/core/**/*.js</include>
                 <include>src/main/javascript/plugins/**/*.js</include>
       </includes>

BTW, yui-compressor-maven-plugin is handling it (http://alchim.sourceforge.net/yuicompressor-maven-plugin/ex_aggregation.html) and I was having a look to migrate aggregation on wisdom underlying solution.
For minification, I'm still having a look because yui seems to do a best job.

Contributor

jbrey commented Apr 22, 2015

ok for the specific case of aggregation.
We could think of an alternated solution where the list of can compose the order of the aggreation then:

       <includes>
                 <include>src/main/javascript/core/the-first-to-aggregate.js</include>
                 <include>src/main/javascript/core/**/*.js</include>
                 <include>src/main/javascript/plugins/**/*.js</include>
       </includes>

BTW, yui-compressor-maven-plugin is handling it (http://alchim.sourceforge.net/yuicompressor-maven-plugin/ex_aggregation.html) and I was having a look to migrate aggregation on wisdom underlying solution.
For minification, I'm still having a look because yui seems to do a best job.

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 22, 2015

Contributor

or at least giving the excludes with patterns so that the file order of the exclusion is not important.

Contributor

jbrey commented Apr 22, 2015

or at least giving the excludes with patterns so that the file order of the exclusion is not important.

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 22, 2015

Member

We tried wildcards and had lots of trouble to debug because it makes the build not reproducible (variables not defined, function not defined...). So it works if you are absolutely sure that the machine building the project is always the same.

One solution would be to use the 'requires.js' aggregator using the module dependencies to get the right order.

Member

cescoffier commented Apr 22, 2015

We tried wildcards and had lots of trouble to debug because it makes the build not reproducible (variables not defined, function not defined...). So it works if you are absolutely sure that the machine building the project is always the same.

One solution would be to use the 'requires.js' aggregator using the module dependencies to get the right order.

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 22, 2015

Contributor

ok then :(

and for the compilation and minification ?

Contributor

jbrey commented Apr 22, 2015

ok then :(

and for the compilation and minification ?

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 22, 2015

Member

For the others two, no problem.

Member

cescoffier commented Apr 22, 2015

For the others two, no problem.

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 22, 2015

Member

we just need to determine the 'configuration' format

Member

cescoffier commented Apr 22, 2015

we just need to determine the 'configuration' format

@cescoffier cescoffier added the planned label Apr 24, 2015

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 24, 2015

Contributor

As requested, here is a pattern format example:

                        <aggregations>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common.min.css</output>
                                <includes>
                                    <include>**/*-min.css</include>
                                </includes>
                                <excludes>
                                    <exclude>**/ie9/*.css</exclude>
                                </excludes>
                            </aggregation>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common.css</output>
                                <includes>
                                    <include>**/*.css</include>
                                </includes>
                                <excludes>
                                    <exclude>*-min.css</exclude>
                                    <exclude>*.min.css</exclude>
                                    <exclude>**/ie9/*.css</exclude>
                                </excludes>
                            </aggregation>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common-ie9.min.css</output>
                                <includes>
                                    <include>**/ie9/*-min.css</include>
                                </includes>
                            </aggregation>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common-ie9.css</output>
                                <includes>
                                    <include>**/ie9/*.css</include>
                                </includes>
                            </aggregation>
                        </aggregations>
Contributor

jbrey commented Apr 24, 2015

As requested, here is a pattern format example:

                        <aggregations>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common.min.css</output>
                                <includes>
                                    <include>**/*-min.css</include>
                                </includes>
                                <excludes>
                                    <exclude>**/ie9/*.css</exclude>
                                </excludes>
                            </aggregation>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common.css</output>
                                <includes>
                                    <include>**/*.css</include>
                                </includes>
                                <excludes>
                                    <exclude>*-min.css</exclude>
                                    <exclude>*.min.css</exclude>
                                    <exclude>**/ie9/*.css</exclude>
                                </excludes>
                            </aggregation>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common-ie9.min.css</output>
                                <includes>
                                    <include>**/ie9/*-min.css</include>
                                </includes>
                            </aggregation>
                            <aggregation>
                                <removeIncluded>true</removeIncluded>
                                <output>${project.build.directory}/classes/assets/common-ie9.css</output>
                                <includes>
                                    <include>**/ie9/*.css</include>
                                </includes>
                            </aggregation>
                        </aggregations>

@cescoffier cescoffier added ready and removed planned labels Apr 26, 2015

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 26, 2015

Member

Ok, after discussion we decided to support wildcard with "warnings".

The aggregation object will let you have an fileset (http://maven.apache.org/ref/3.0.3/maven-model/apidocs/org/apache/maven/model/FileSet.html). The user is responsible to set the right directory, and need to avoid order issue.

Member

cescoffier commented Apr 26, 2015

Ok, after discussion we decided to support wildcard with "warnings".

The aggregation object will let you have an fileset (http://maven.apache.org/ref/3.0.3/maven-model/apidocs/org/apache/maven/model/FileSet.html). The user is responsible to set the right directory, and need to avoid order issue.

@cescoffier cescoffier added this to the 0.8.1 milestone Apr 26, 2015

@cescoffier cescoffier removed the in progress label Apr 26, 2015

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 27, 2015

Contributor

Thanks!
I'll try to have a look soon

Contributor

jbrey commented Apr 27, 2015

Thanks!
I'll try to have a look soon

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 28, 2015

Contributor

ok, I've tried.

In fact the aggregation with patterns in fileset does not help as it does not take into account the order of the definition, which would resolve the deterministic issue.

Let's say that I have these files:

   - src/main/javascript
          - core
                  - a.js
                  - the-first-to-aggregate.js
           - plugins
                  - a-file-that-uses-the-very-useful-class-in-utils.js
                  - another-file.js
           - utils
                   - a-very-useful-class.js

and this pom configuration:

  <includes>
                 <include>src/main/javascript/core/the-first-to-aggregate.js</include>
                 <include>src/main/javascript/core/**/*.js</include>
                 <include>src/main/javascript/utils/**/*.js</include>
                 <include>src/main/javascript/plugins/**/*.js</include>
  </includes>

I was expecting to have the aggregation by this order:

   - src/main/javascript/core/the-first-to-aggregate.js
   - src/main/javascript/core/a.js
   - src/main/javascript/utils/a-very-useful-class.js
   - src/main/javascript/plugins/a-file-that-uses-the-very-useful-class-in-utils.js
   - src/main/javascript/plugins/another-file.js

but instead the FileSetManager takes all the includes files and sort them so that the aggregation ends in the following order:

   - src/main/javascript/core/a.js
   - src/main/javascript/core/the-first-to-aggregate.js
   - src/main/javascript/plugins/a-file-that-uses-the-very-useful-class-in-utils.js
   - src/main/javascript/plugins/another-file.js
   - src/main/javascript/utils/a-very-useful-class.js

Thoughts ?

Thanks
JB

Contributor

jbrey commented Apr 28, 2015

ok, I've tried.

In fact the aggregation with patterns in fileset does not help as it does not take into account the order of the definition, which would resolve the deterministic issue.

Let's say that I have these files:

   - src/main/javascript
          - core
                  - a.js
                  - the-first-to-aggregate.js
           - plugins
                  - a-file-that-uses-the-very-useful-class-in-utils.js
                  - another-file.js
           - utils
                   - a-very-useful-class.js

and this pom configuration:

  <includes>
                 <include>src/main/javascript/core/the-first-to-aggregate.js</include>
                 <include>src/main/javascript/core/**/*.js</include>
                 <include>src/main/javascript/utils/**/*.js</include>
                 <include>src/main/javascript/plugins/**/*.js</include>
  </includes>

I was expecting to have the aggregation by this order:

   - src/main/javascript/core/the-first-to-aggregate.js
   - src/main/javascript/core/a.js
   - src/main/javascript/utils/a-very-useful-class.js
   - src/main/javascript/plugins/a-file-that-uses-the-very-useful-class-in-utils.js
   - src/main/javascript/plugins/another-file.js

but instead the FileSetManager takes all the includes files and sort them so that the aggregation ends in the following order:

   - src/main/javascript/core/a.js
   - src/main/javascript/core/the-first-to-aggregate.js
   - src/main/javascript/plugins/a-file-that-uses-the-very-useful-class-in-utils.js
   - src/main/javascript/plugins/another-file.js
   - src/main/javascript/utils/a-very-useful-class.js

Thoughts ?

Thanks
JB

@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey Apr 28, 2015

Contributor

BTW, in fact I didn't checked who is doing the sort, if it's the FileSetManager, wisdom or google closure.

Contributor

jbrey commented Apr 28, 2015

BTW, in fact I didn't checked who is doing the sort, if it's the FileSetManager, wisdom or google closure.

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 30, 2015

Member

It's the Maven File Set Manager. That's the main issue with wildcards (see one of my previous comment) -> it creates non-determism

Member

cescoffier commented Apr 30, 2015

It's the Maven File Set Manager. That's the main issue with wildcards (see one of my previous comment) -> it creates non-determism

@cescoffier

This comment has been minimized.

Show comment
Hide comment
@cescoffier

cescoffier Apr 30, 2015

Member

In addition there is a bug in the logged message using getFiles and not the set of selected file

Member

cescoffier commented Apr 30, 2015

In addition there is a bug in the logged message using getFiles and not the set of selected file

@cescoffier cescoffier closed this in b22dc28 May 2, 2015

@cescoffier cescoffier removed the in progress label May 2, 2015

cescoffier added a commit that referenced this issue May 3, 2015

Fix issue related to #480
Use fileSet instead of fileset to be homogeneous.

Signed-off-by: Clement Escoffier <clement.escoffier@gmail.com>
@jbrey

This comment has been minimized.

Show comment
Hide comment
@jbrey

jbrey May 22, 2015

Contributor

Hi Clement,

I finally manage to test it a bit and ... great !!! it seems to work well.

Big thanks.
JB

PS: on javascript seems ok as expected, on css I'm not sure but it seems that if a file is included a first time explicitly and included in a more generic pattern after, file is included twice. I didn't check twice and I don't have the need anymore...

Contributor

jbrey commented May 22, 2015

Hi Clement,

I finally manage to test it a bit and ... great !!! it seems to work well.

Big thanks.
JB

PS: on javascript seems ok as expected, on css I'm not sure but it seems that if a file is included a first time explicitly and included in a more generic pattern after, file is included twice. I didn't check twice and I don't have the need anymore...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment