Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.6, 4.0-ALPHA
    • Fix Version/s: 3.6, 4.0-ALPHA
    • Component/s: general/build
    • Labels:
      None

      Description

      In trunk, the check-legal-lucene ant target is not checking any lucene/contrib/**/lib/ directories; the modules/**/lib/ directories are not being checked; and check-legal-solr can't be checking solr/example/lib/**/*.jar, because there are currently .jar files in there that don't have a license.

      These targets are set up to take in a full list of lib/ directories in which to check, but modules move around, and these lists are not being kept up-to-date.

      Instead, check-legal-* should run for each module, if the module has a lib/ directory, and it should be specialized for modules that have more than one (solr/core/) or that have a lib/ directory in a non-standard place (lucene/core/).

      1. backport.patch
        85 kB
        Dawid Weiss
      2. LUCENE3774.patch
        94 kB
        Dawid Weiss
      3. LUCENE-3774.patch
        96 kB
        Dawid Weiss
      4. LUCENE-3774.patch
        94 kB
        Dawid Weiss
      5. LUCENE-3774.patch
        84 kB
        Dawid Weiss
      6. LUCENE-3774.patch
        84 kB
        Dawid Weiss
      7. LUCENE-3774.patch
        7 kB
        Steve Rowe
      8. LUCENE-3774.patch
        7 kB
        Steve Rowe

        Issue Links

          Activity

          Hide
          Steve Rowe added a comment -

          Patch:

          • Eliminates check-legal-solr and renames check-legal-lucene to check-legal.
          • ant test depends on either validate-lucene or validate-solr, which depend on check-legal.
          • check-legal takes no action unless there is a lib/ directory in the module where it's being run.
          • lucene/core/ and solr/core/ modules specialize check-legal to handle their non-standard lib/ directories.
          Show
          Steve Rowe added a comment - Patch: Eliminates check-legal-solr and renames check-legal-lucene to check-legal . ant test depends on either validate-lucene or validate-solr , which depend on check-legal . check-legal takes no action unless there is a lib/ directory in the module where it's being run. lucene/core/ and solr/core/ modules specialize check-legal to handle their non-standard lib/ directories.
          Hide
          Steve Rowe added a comment -

          This version of the patch fixes the previous version's incorrect spelling of the example-DIH/**/lib/ directories.

          Show
          Steve Rowe added a comment - This version of the patch fixes the previous version's incorrect spelling of the example-DIH/**/lib/ directories.
          Hide
          Mark Miller added a comment -

          Thanks for taking this on Steve!

          Show
          Mark Miller added a comment - Thanks for taking this on Steve!
          Hide
          Dawid Weiss added a comment -

          I looked at DependencyChecker, we should rewrite it as an ANT task, really – it would be cleaner to use (with a dirset or a fileset). I'm thinking it could look like this:

          <check-license>
           <jars>
            <fileset dir="base" includes="**/lib/*.jar" />
           </jars>
          </check-license>
          

          Base JAR names could be considered a match (or better, a mapper could be added to specify explicit mapping between the two). The upside of this is that we could have a single top-level check license task that would simply scan everything, once and wouldn't need to be updated as libs are added or removed.

          I can help with the implementation, this is a trivial task, but I'd appreciate if you could hook it in ANT scripts where appropriate, Steve. Let me know if this is a good idea.

          Show
          Dawid Weiss added a comment - I looked at DependencyChecker, we should rewrite it as an ANT task, really – it would be cleaner to use (with a dirset or a fileset). I'm thinking it could look like this: <check-license> <jars> <fileset dir= "base" includes= "**/lib/*.jar" /> </jars> </check-license> Base JAR names could be considered a match (or better, a mapper could be added to specify explicit mapping between the two). The upside of this is that we could have a single top-level check license task that would simply scan everything, once and wouldn't need to be updated as libs are added or removed. I can help with the implementation, this is a trivial task, but I'd appreciate if you could hook it in ANT scripts where appropriate, Steve. Let me know if this is a good idea.
          Hide
          Dawid Weiss added a comment -

          I wrote a task as a quick experiment, it looks like this:

          <license-check>
            <fileset dir="../..">
              <include name="**/*.jar" />
              <!-- Speed up scanning a bit. -->
              <exclude name="**/src/**" />
              <exclude name="**/build/**" />
            </fileset>
          
            <licenseMapper>
              <filtermapper>
                <replacestring from="\" to="/" />
                <replaceregex pattern="\.jar$" replace="" flags="gi" />
          
                <!-- Handle non-typical version patterns. -->
                <replaceregex pattern="/cpptasks([^/]+)$"            replace="/cpptasks" flags="gi" />
                <replaceregex pattern="/commons-csv-([^/]+)$"        replace="/commons-csv" flags="gi" />
                <replaceregex pattern="/apache-solr-noggit-([^/]+)$" replace="/apache-solr-noggit" flags="gi" />
          
                <!-- Typical version trailers. -->
                <replaceregex pattern="\-(r)?([0-9\-\.])+(beta([0-9\-\.])*)?$" replace="" flags="gi" />
              </filtermapper>
            </licenseMapper>
          </license-check>
          

          The mapper tells where a LICENSE and NOTICE file should be expected for each JAR. This is different from the current design where LICENSE and NOTICE files are matched against each other, but it opens up a few interesting possibilities:

          • exact reporting where a LICENSE/NOTICE file is expected for a given JAR (with full expected filename),
          • placing all LICENSE/NOTICE files in a single location in the project (not alongside JARs),
          • allowing a single LICENSE/NOTICE to cover multiple JARs (for example slf4j.LICENSE would cover all slf4j JAR files).

          Let me know what you think.

          Show
          Dawid Weiss added a comment - I wrote a task as a quick experiment, it looks like this: <license-check> <fileset dir= "../.." > <include name= "**/*.jar" /> <!-- Speed up scanning a bit. --> <exclude name= "**/src/**" /> <exclude name= "**/build/**" /> </fileset> <licenseMapper> <filtermapper> <replacestring from= "\" to= "/" /> <replaceregex pattern= "\.jar$" replace= "" flags=" gi" /> <!-- Handle non-typical version patterns. --> <replaceregex pattern= "/cpptasks([^/]+)$" replace= "/cpptasks" flags= "gi" /> <replaceregex pattern= "/commons-csv-([^/]+)$" replace= "/commons-csv" flags= "gi" /> <replaceregex pattern= "/apache-solr-noggit-([^/]+)$" replace= "/apache-solr-noggit" flags= "gi" /> <!-- Typical version trailers. --> <replaceregex pattern= "\-(r)?([0-9\-\.])+(beta([0-9\-\.])*)?$" replace= "" flags=" gi" /> </filtermapper> </licenseMapper> </license-check> The mapper tells where a LICENSE and NOTICE file should be expected for each JAR. This is different from the current design where LICENSE and NOTICE files are matched against each other, but it opens up a few interesting possibilities: exact reporting where a LICENSE/NOTICE file is expected for a given JAR (with full expected filename), placing all LICENSE/NOTICE files in a single location in the project (not alongside JARs), allowing a single LICENSE/NOTICE to cover multiple JARs (for example slf4j.LICENSE would cover all slf4j JAR files). Let me know what you think.
          Hide
          Dawid Weiss added a comment -

          An example output:

          [license-check] MISSING LICENSE for the following file:
          [license-check] C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-1.5.2.jar
          [license-check] Expected locations below:
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-ASL.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-BSD.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-BSD_LIKE.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-CDDL.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-CPL.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-MIT.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-MPL.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-PD.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-SUN.txt
          [license-check]   => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-FAKE.txt
          ...
          [license-check] Scanned 91 JAR file(s) for licenses, 15 error(s).
          Exception in thread "main" C:\Work\lucene-solr\lucene\tools\build.xml:47: License check failed. Check the logs.
          
          Show
          Dawid Weiss added a comment - An example output: [license-check] MISSING LICENSE for the following file: [license-check] C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-1.5.2.jar [license-check] Expected locations below: [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-ASL.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-BSD.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-BSD_LIKE.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-CDDL.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-CPL.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-MIT.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-MPL.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-PD.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-SUN.txt [license-check] => C:\Work\lucene-solr\modules\analysis\morfologik\lib\morfologik-polish-LICENSE-FAKE.txt ... [license-check] Scanned 91 JAR file(s) for licenses, 15 error(s). Exception in thread "main" C:\Work\lucene-solr\lucene\tools\build.xml:47: License check failed. Check the logs.
          Hide
          Robert Muir added a comment -

          The upside of this is that we could have a single top-level check license task that would simply scan everything, once and wouldn't need to be updated as libs are added or removed.

          I think with steve's patch this is per-module? I woudl really prefer it done per-module and not as a 'top-level' task.

          If its a top-level task, then test cannot depend on it. I don't want to scan licenses of all of lucene/solr just to run 'ant test -Dtestcase' within some confined space, or to just run lucene core tests.

          Show
          Robert Muir added a comment - The upside of this is that we could have a single top-level check license task that would simply scan everything, once and wouldn't need to be updated as libs are added or removed. I think with steve's patch this is per-module? I woudl really prefer it done per-module and not as a 'top-level' task. If its a top-level task, then test cannot depend on it. I don't want to scan licenses of all of lucene/solr just to run 'ant test -Dtestcase' within some confined space, or to just run lucene core tests.
          Hide
          Dawid Weiss added a comment -

          I have a different view on this. Things like this (license checking) are typically integration tests. Having them per-module only complicates build files and is an unnecessary overhead for running normal tests (because dependencies change rarely). Integration tests are usually long running and tedious and a build server is a nice facility to run them on (and this justifies why they could be placed on the top level).

          Show
          Dawid Weiss added a comment - I have a different view on this. Things like this (license checking) are typically integration tests. Having them per-module only complicates build files and is an unnecessary overhead for running normal tests (because dependencies change rarely). Integration tests are usually long running and tedious and a build server is a nice facility to run them on (and this justifies why they could be placed on the top level).
          Hide
          Dawid Weiss added a comment -

          I'll finish the implementation and add to this issue as a patch. The task can be used per-module (even if I don't see the point of this), not necessarily at the top level (it's just a matter of the fileset/ mapper used).

          Show
          Dawid Weiss added a comment - I'll finish the implementation and add to this issue as a patch. The task can be used per-module (even if I don't see the point of this), not necessarily at the top level (it's just a matter of the fileset/ mapper used).
          Hide
          Robert Muir added a comment -

          The task can be used per-module (even if I don't see the point of this), not necessarily at the top level (it's just a matter of the fileset/ mapper used).

          There are a few advantages: having compile/test/javadocs/licensecheck/... per-module in the common-build means that the entire definition for a module is pretty self-contained.

          It means we can easily add/disable modules without editing many 'global' files.
          In most cases a module won't have to specify anything, the default task looking at all jar files in /lib? would be fine.

          On the other hand, a top-level task that looks for all jar files is going to be more complicated,
          for example it will fail if it does not have logic to ignore start.jar/post.jar in the solr example (no idea what other exceptions may exist).

          Show
          Robert Muir added a comment - The task can be used per-module (even if I don't see the point of this), not necessarily at the top level (it's just a matter of the fileset/ mapper used). There are a few advantages: having compile/test/javadocs/licensecheck/... per-module in the common-build means that the entire definition for a module is pretty self-contained. It means we can easily add/disable modules without editing many 'global' files. In most cases a module won't have to specify anything, the default task looking at all jar files in /lib? would be fine. On the other hand, a top-level task that looks for all jar files is going to be more complicated, for example it will fail if it does not have logic to ignore start.jar/post.jar in the solr example (no idea what other exceptions may exist).
          Hide
          Dawid Weiss added a comment -

          Sure there are pros and cons. I just tend to disagree with you about which solution is better. Having a few top-level exclusion rules in the fileset (build folders, example folders) is not that bad to me. Having it automatically detect missing files in modules and other locations is a big plus that to me makes the decision easy. I just executed it from the top checkout and it already got some inconsistencies:

          [license-check] MISSING NOTICE for the license file:
          [license-check]   C:\Work\lucene-solr\lucene\contrib\misc\ant_lib\cpptasks-LICENSE-ASL.txt
          [license-check]   Expected location below:
          [license-check]   C:\Work\lucene-solr\lucene\contrib\misc\ant_lib\cpptasks-NOTICE.txt
          
          [there is only one shared notice for ant and ant-junit?]
          [license-check] MISSING NOTICE for the license file:
          [license-check]   C:\Work\lucene-solr\lucene\lib\ant-junit-LICENSE-ASL.txt
          [license-check]   Expected location below:
          [license-check]   C:\Work\lucene-solr\lucene\lib\ant-junit-NOTICE.txt
          

          It means we can easily add/disable modules without editing many 'global' files.

          Since the detection is global this isn't an argument because any JAR you add will just be included in the global detection. We can restrict its pattern to follow */lib/* to simulate previous behavior but as the above output shows (ant_lib) it may be wiser to just respond to build failures resulting from inconsistencies as they arise?

          Show
          Dawid Weiss added a comment - Sure there are pros and cons. I just tend to disagree with you about which solution is better. Having a few top-level exclusion rules in the fileset (build folders, example folders) is not that bad to me. Having it automatically detect missing files in modules and other locations is a big plus that to me makes the decision easy. I just executed it from the top checkout and it already got some inconsistencies: [license-check] MISSING NOTICE for the license file: [license-check] C:\Work\lucene-solr\lucene\contrib\misc\ant_lib\cpptasks-LICENSE-ASL.txt [license-check] Expected location below: [license-check] C:\Work\lucene-solr\lucene\contrib\misc\ant_lib\cpptasks-NOTICE.txt [there is only one shared notice for ant and ant-junit?] [license-check] MISSING NOTICE for the license file: [license-check] C:\Work\lucene-solr\lucene\lib\ant-junit-LICENSE-ASL.txt [license-check] Expected location below: [license-check] C:\Work\lucene-solr\lucene\lib\ant-junit-NOTICE.txt It means we can easily add/disable modules without editing many 'global' files. Since the detection is global this isn't an argument because any JAR you add will just be included in the global detection. We can restrict its pattern to follow * /lib/ * to simulate previous behavior but as the above output shows (ant_lib) it may be wiser to just respond to build failures resulting from inconsistencies as they arise?
          Hide
          Dawid Weiss added a comment -

          A patch with an ant task implementing *LICENSE/NOTICE checks, modified build files (no submodule checking, top-level ant validate does everything) and fixes to current LICENSE/NOTICE quirks and inconsistencies.

          Show
          Dawid Weiss added a comment - A patch with an ant task implementing *LICENSE/NOTICE checks, modified build files (no submodule checking, top-level ant validate does everything) and fixes to current LICENSE/NOTICE quirks and inconsistencies.
          Hide
          Dawid Weiss added a comment -

          I've attached a patch and pushed the branch to github (if you're lazy):
          https://github.com/dweiss/lucene_solr/tree/LUCENE3774

          The big upside to me is this (top level):

          > ant validate
          ...
          validate:
          [license-check] Scanned 82 JAR file(s) for licenses (in 0.12s.), 0 error(s).
          
          BUILD SUCCESSFUL
          Total time: 2 seconds
          
          Show
          Dawid Weiss added a comment - I've attached a patch and pushed the branch to github (if you're lazy): https://github.com/dweiss/lucene_solr/tree/LUCENE3774 The big upside to me is this (top level): > ant validate ... validate: [license-check] Scanned 82 JAR file(s) for licenses (in 0.12s.), 0 error(s). BUILD SUCCESSFUL Total time: 2 seconds
          Hide
          Dawid Weiss added a comment -

          You can add verbose="true" to check-licenses task or run with ant -verbose and it'll show you the details of where each license file is.

          This patch is a seed for discussion; feel free to tear it apart and go back to per-module checks (but I really think that 0.12 sec. may be worth an occasional top-level update). I currently excluded SOLR examples/ (although some files in there do live with license/ notice information).

          Show
          Dawid Weiss added a comment - You can add verbose="true" to check-licenses task or run with ant -verbose and it'll show you the details of where each license file is. This patch is a seed for discussion; feel free to tear it apart and go back to per-module checks (but I really think that 0.12 sec. may be worth an occasional top-level update). I currently excluded SOLR examples/ (although some files in there do live with license/ notice information).
          Hide
          Steve Rowe added a comment -

          Hi Dawid,

          I agree with you that a top-level validate task would be better, though we'll need to insure that it gets invoked regularly on Jenkins.

          I can't apply your patch because it's in Git format - I don't use Git - can you generate a Subversion-compatible patch?

          Thanks,
          Steve

          Show
          Steve Rowe added a comment - Hi Dawid, I agree with you that a top-level validate task would be better, though we'll need to insure that it gets invoked regularly on Jenkins. I can't apply your patch because it's in Git format - I don't use Git - can you generate a Subversion-compatible patch? Thanks, Steve
          Hide
          Steve Rowe added a comment -

          Dawid, I just skimmed your patch and I see that you've done lots of license file cleanups - cool!

          Heads up: I plan on committing SOLR-3123 today, and I need to add the Eclipse Public License to LicenseType.java, where your patch also makes changes.

          Show
          Steve Rowe added a comment - Dawid, I just skimmed your patch and I see that you've done lots of license file cleanups - cool! Heads up: I plan on committing SOLR-3123 today, and I need to add the Eclipse Public License to LicenseType.java, where your patch also makes changes.
          Hide
          Robert Muir added a comment -

          Having a few top-level exclusion rules in the fileset (build folders, example folders) is not that bad to me.

          Keep in mind also, that when I see things like ".." from lucene/, I know this cannot totally work,
          because lucene/ must be buildable/packagable by itself (its broken in trunk that this doesn't work today,
          mostly because hudson does not test this is the case: LUCENE-2974).

          Show
          Robert Muir added a comment - Having a few top-level exclusion rules in the fileset (build folders, example folders) is not that bad to me. Keep in mind also, that when I see things like ".." from lucene/, I know this cannot totally work, because lucene/ must be buildable/packagable by itself (its broken in trunk that this doesn't work today, mostly because hudson does not test this is the case: LUCENE-2974 ).
          Hide
          Dawid Weiss added a comment -

          Hi Steven. I will merge the changes, no problem. As for committing this in, let's wait until folks have some time to express if they prefer per-module or global checks. I left per-module validation hooks in place as far as I remember, so we could move it down from top-level if needed. It shouldn't even affect performance that much; I just think it's cleaner if done at top-level, that's all.

          As for Jenkins: if builds are done per-subproject (Lucene, Solr) then indeed this validation wouldn't be executed. I imagine at least one plan should run full top-level build though so something would fire a build exception in case of validation errors?

          As for git vs. svn patch – I've looked at this: http://stackoverflow.com/questions/3418277/how-to-apply-git-diff-patch

          I'll re-generate the patch.

          Show
          Dawid Weiss added a comment - Hi Steven. I will merge the changes, no problem. As for committing this in, let's wait until folks have some time to express if they prefer per-module or global checks. I left per-module validation hooks in place as far as I remember, so we could move it down from top-level if needed. It shouldn't even affect performance that much; I just think it's cleaner if done at top-level, that's all. As for Jenkins: if builds are done per-subproject (Lucene, Solr) then indeed this validation wouldn't be executed. I imagine at least one plan should run full top-level build though so something would fire a build exception in case of validation errors? As for git vs. svn patch – I've looked at this: http://stackoverflow.com/questions/3418277/how-to-apply-git-diff-patch I'll re-generate the patch.
          Hide
          Dawid Weiss added a comment -

          git patch with --no-prefix?

          Show
          Dawid Weiss added a comment - git patch with --no-prefix?
          Hide
          Dawid Weiss added a comment -

          because lucene/ must be buildable/packagable by itself

          Does this apply to solr as well? I don't see how this can work with shared tools (like DependencyChecker previously). If it's so important to have separate build files and separate infrastructure for solr and lucene then I don't see the point of having the global build file, really.

          I see things like ".." from lucene/, I know this cannot totally work,

          Where did you see it? If you mean target named "test" then this is only for debugging, I should have removed it.

          Show
          Dawid Weiss added a comment - because lucene/ must be buildable/packagable by itself Does this apply to solr as well? I don't see how this can work with shared tools (like DependencyChecker previously). If it's so important to have separate build files and separate infrastructure for solr and lucene then I don't see the point of having the global build file, really. I see things like ".." from lucene/, I know this cannot totally work, Where did you see it? If you mean target named " test " then this is only for debugging, I should have removed it.
          Hide
          Dawid Weiss added a comment -

          Ok, so now Solr reaches out to Lucene's folders, but it shouldn't be the other way around, right? I can update the patch to have a macro for dependency checking (on Lucene side) and apply it on the Solr side too. This will make validation execute in two places instead of one, but it'll keep Lucene self-contained.

          Show
          Dawid Weiss added a comment - Ok, so now Solr reaches out to Lucene's folders, but it shouldn't be the other way around, right? I can update the patch to have a macro for dependency checking (on Lucene side) and apply it on the Solr side too. This will make validation execute in two places instead of one, but it'll keep Lucene self-contained.
          Hide
          Uwe Schindler added a comment -

          Dawid: Nice Task implementation, I like it, much easier than the static main()!

          Currently we require ANT 1.7 (and hudson/myself always uses ANT 1.7), so: Do we need to upgrade? As far as I remember, ANT changed in 1.8 for the first time to use only ResourceSets, in previous versions your Task would only use the FileSet class. As it does not work with non-file resources at all, should we better use FileSet in the java code? - On the other hand, I am not sure about regexmapper support, 1.7 only supports globmapper?

          If we decide to go to ANT 1.8, don't forget to update BUILD.txt!

          Show
          Uwe Schindler added a comment - Dawid: Nice Task implementation, I like it, much easier than the static main()! Currently we require ANT 1.7 (and hudson/myself always uses ANT 1.7), so: Do we need to upgrade? As far as I remember, ANT changed in 1.8 for the first time to use only ResourceSets, in previous versions your Task would only use the FileSet class. As it does not work with non-file resources at all, should we better use FileSet in the java code? - On the other hand, I am not sure about regexmapper support, 1.7 only supports globmapper? If we decide to go to ANT 1.8, don't forget to update BUILD.txt!
          Hide
          Steve Rowe added a comment - - edited

          I'll re-generate the patch.

          git patch with --no-prefix?

          Thanks! This patch worked for me (using patch -p0 < LUCENE-3774.patch).

          I've looked at this: http://stackoverflow.com/questions/3418277/how-to-apply-git-diff-patch

          I tried another suggestion from this web page, using your original patch: patch -p1 < LUCENE-3774.patch. This also worked, so there was no reason to re-generate after all - sorry about that.

          Currently we require ANT 1.7 (and hudson/myself always uses ANT 1.7), so: Do we need to upgrade?

          I use Ant 1.7.1, and Dawid's patch worked for me, so I don't think we need to upgrade?

          After applying the patch, when I run ant validate at the top level, I get the following:

          validate:
          [license-check] MISSING NOTICE for the license file:
          [license-check]   C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-LICENSE-ASL.txt
          [license-check]   Expected location below:
          [license-check]   C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-NOTICE.txt
          [license-check] Scanned 82 JAR file(s) for licenses (in 4.71s.), 0 error(s).
          

          This tooks significantly longer for me than for you, Dawid: 4.71s vs. 0.12s (maybe disk caching effects? I have an SSD...). Even so, it's fast enough not to cause trouble.

          Show
          Steve Rowe added a comment - - edited I'll re-generate the patch. git patch with --no-prefix? Thanks! This patch worked for me (using patch -p0 < LUCENE-3774.patch ). I've looked at this: http://stackoverflow.com/questions/3418277/how-to-apply-git-diff-patch I tried another suggestion from this web page, using your original patch: patch -p1 < LUCENE-3774.patch . This also worked, so there was no reason to re-generate after all - sorry about that. Currently we require ANT 1.7 (and hudson/myself always uses ANT 1.7), so: Do we need to upgrade? I use Ant 1.7.1, and Dawid's patch worked for me, so I don't think we need to upgrade? After applying the patch, when I run ant validate at the top level, I get the following: validate: [license-check] MISSING NOTICE for the license file: [license-check] C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-LICENSE-ASL.txt [license-check] Expected location below: [license-check] C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-NOTICE.txt [license-check] Scanned 82 JAR file(s) for licenses (in 4.71s.), 0 error(s). This tooks significantly longer for me than for you, Dawid: 4.71s vs. 0.12s (maybe disk caching effects? I have an SSD...). Even so, it's fast enough not to cause trouble.
          Hide
          Dawid Weiss added a comment -

          Uwe: I'm not sure if I used anything that requires Ant 1.8+ – will double check before committing once we finalize the patch, thanks for noticing.

          This tooks significantly longer for me than for you, Dawid: 4.71s vs. 0.12s (maybe disk caching effects? I have an SSD...). Even so, it's fast enough not to cause trouble.

          I have a fairly fast SSD and indeed, this was most likely a result of warmed-up caches. 5 seconds seems longish to me though because there's really not that much to do. I'll add some diagnostics maybe so that we can see what takes so long (scanning vs. license checks later on?).

          Show
          Dawid Weiss added a comment - Uwe: I'm not sure if I used anything that requires Ant 1.8+ – will double check before committing once we finalize the patch, thanks for noticing. This tooks significantly longer for me than for you, Dawid: 4.71s vs. 0.12s (maybe disk caching effects? I have an SSD...). Even so, it's fast enough not to cause trouble. I have a fairly fast SSD and indeed, this was most likely a result of warmed-up caches. 5 seconds seems longish to me though because there's really not that much to do. I'll add some diagnostics maybe so that we can see what takes so long (scanning vs. license checks later on?).
          Hide
          Dawid Weiss added a comment -

          Steve what computer are you working on? I just checked after rebooting (no caches). ant validate took 2 seconds (precompiled), but license scanning was pretty much like before:

          [license-check] Scanned 82 JAR file(s) for licenses (in 0.27s.), 0 error(s).
          

          This is odd, shouldn't be an order of magnitude difference.

          Show
          Dawid Weiss added a comment - Steve what computer are you working on? I just checked after rebooting (no caches). ant validate took 2 seconds (precompiled), but license scanning was pretty much like before: [license-check] Scanned 82 JAR file(s) for licenses (in 0.27s.), 0 error(s). This is odd, shouldn't be an order of magnitude difference.
          Hide
          Dawid Weiss added a comment -

          I know whom to blame. I used ant 1.7.1 and the result is:

          [license-check] Scanned 82 JAR file(s) for licenses (in 4.44s.), 0 error(s).
          

          Try with the latest Ant (1.8.2). I'll try to see what's causing such a huge speed difference, but maybe we should just enforce Ant 1.8.+?

          Show
          Dawid Weiss added a comment - I know whom to blame. I used ant 1.7.1 and the result is: [license-check] Scanned 82 JAR file(s) for licenses (in 4.44s.), 0 error(s). Try with the latest Ant (1.8.2). I'll try to see what's causing such a huge speed difference, but maybe we should just enforce Ant 1.8.+?
          Hide
          Steve Rowe added a comment - - edited

          I know whom to blame. I used ant 1.7.1 and the result is: [...] 4.44s.

          Cool, glad you tracked it down.

          Try with the latest Ant (1.8.2). I'll try to see what's causing such a huge speed difference, but maybe we should just enforce Ant 1.8.+?

          I recall there being problems with Ant 1.8 in some parts of the build - sorry, I don't remember the details. I'm sure we could work through those issues, whatever they are, though, so +1 from me to require Ant 1.8.

          With Ant 1.8.2, I get:

          [license-check] Scanned 82 JAR file(s) for licenses (in 0.21s.), 0 error(s).
          

          So yeah, Ant 1.7.1 is the culprit.

          Show
          Steve Rowe added a comment - - edited I know whom to blame. I used ant 1.7.1 and the result is: [...] 4.44s. Cool, glad you tracked it down. Try with the latest Ant (1.8.2). I'll try to see what's causing such a huge speed difference, but maybe we should just enforce Ant 1.8.+? I recall there being problems with Ant 1.8 in some parts of the build - sorry, I don't remember the details. I'm sure we could work through those issues, whatever they are, though, so +1 from me to require Ant 1.8. With Ant 1.8.2, I get: [license-check] Scanned 82 JAR file(s) for licenses (in 0.21s.), 0 error(s). So yeah, Ant 1.7.1 is the culprit.
          Hide
          Uwe Schindler added a comment -

          ANT 1.8 improved performance of <fileSet/>, I think I read that in the bulletin.

          But what we have seen: Dawid's code is compatible with ANT 1.7, so we dont need to change minimum requirements. If its slower or not should not force us to upgrade. So build works, all fine. -1 to require ANT 1.8.

          Show
          Uwe Schindler added a comment - ANT 1.8 improved performance of <fileSet/>, I think I read that in the bulletin. But what we have seen: Dawid's code is compatible with ANT 1.7, so we dont need to change minimum requirements. If its slower or not should not force us to upgrade. So build works, all fine. -1 to require ANT 1.8.
          Hide
          Dawid Weiss added a comment -

          Yep, sure, this isn't crucial by any means. I will try to inspect what's causing such a dramatic slowdown though, it isn't normal.

          Show
          Dawid Weiss added a comment - Yep, sure, this isn't crucial by any means. I will try to inspect what's causing such a dramatic slowdown though, it isn't normal.
          Hide
          Dawid Weiss added a comment -

          A cleaned up patch that does split license checking into solr/lucene/modules (3 parts). I also cleaned up modules/build.xml to use a macro and avoid repetition .

          Can you please double check that everything works with Ant 1.7? Weirdly enough, I now see no slowdown on execution...

          Show
          Dawid Weiss added a comment - A cleaned up patch that does split license checking into solr/lucene/modules (3 parts). I also cleaned up modules/build.xml to use a macro and avoid repetition . Can you please double check that everything works with Ant 1.7? Weirdly enough, I now see no slowdown on execution...
          Hide
          Dawid Weiss added a comment -

          Oh, ok. I see the slowdown still, it's just not that big because it's split into three scans.

          Show
          Dawid Weiss added a comment - Oh, ok. I see the slowdown still, it's just not that big because it's split into three scans.
          Hide
          Uwe Schindler added a comment -

          Oh, ok. I see the slowdown still, it's just not that big because it's split into three scans.

          Exactly, that's what I know, ANT somehow exponentially got slower on larger filesets. I think you can reproduce the same slowdown with other ant tasks, too, so its completely unrelated to your Task implementation. We simply have in the Lucene builds no other FileSets like that one (except maybe the main lucene clover task).

          Show
          Uwe Schindler added a comment - Oh, ok. I see the slowdown still, it's just not that big because it's split into three scans. Exactly, that's what I know, ANT somehow exponentially got slower on larger filesets. I think you can reproduce the same slowdown with other ant tasks, too, so its completely unrelated to your Task implementation. We simply have in the Lucene builds no other FileSets like that one (except maybe the main lucene clover task).
          Hide
          Dawid Weiss added a comment -

          That had to be something really ugly in ANT 1.7 – I'll have to check what kind of bug it was, but I bet it was glorious

          Show
          Dawid Weiss added a comment - That had to be something really ugly in ANT 1.7 – I'll have to check what kind of bug it was, but I bet it was glorious
          Hide
          Steve Rowe added a comment -

          Can you please double check that everything works with Ant 1.7? Weirdly enough, I now see no slowdown on execution...

          Works for me (from the top level) under Ant 1.7.1.

          Under solr, two problems are found:

          validate:
               [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\solr
           [licenses] MISSING NOTICE for the license file:
           [licenses]   C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-LICENSE-ASL.txt
           [licenses]   Expected location below:
           [licenses]   C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-NOTICE.txt
           [licenses] MISSING LICENSE for the following file:
           [licenses]   C:\cygwin\home\s\svn\lucene\dev\trunk\solr\example\exampledocs\post.jar
          

          But: When I run ant validate from lucene/ or modules/ or solr/, it fails (lucene/ error shown here):

          BUILD FAILED
          C:\cygwin\home\s\svn\lucene\dev\trunk\lucene\build.xml:178: The following error occurred while executing this line:
          C:\cygwin\home\s\svn\lucene\dev\trunk\lucene\tools\custom-tasks.xml:22: Problem: failed to create task or type licenses
          Cause: The name is undefined.
          Action: Check the spelling.
          Action: Check that any custom tasks/types have been declared.
          Action: Check that any <presetdef>/<macrodef> declarations have taken place.
          
          Show
          Steve Rowe added a comment - Can you please double check that everything works with Ant 1.7? Weirdly enough, I now see no slowdown on execution... Works for me (from the top level) under Ant 1.7.1. Under solr, two problems are found: validate: [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\solr [licenses] MISSING NOTICE for the license file: [licenses] C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-LICENSE-ASL.txt [licenses] Expected location below: [licenses] C:\cygwin\home\s\svn\lucene\dev\trunk\solr\contrib\langid\lib\jsonic-NOTICE.txt [licenses] MISSING LICENSE for the following file: [licenses] C:\cygwin\home\s\svn\lucene\dev\trunk\solr\example\exampledocs\post.jar But : When I run ant validate from lucene/ or modules/ or solr/ , it fails ( lucene/ error shown here): BUILD FAILED C:\cygwin\home\s\svn\lucene\dev\trunk\lucene\build.xml:178: The following error occurred while executing this line: C:\cygwin\home\s\svn\lucene\dev\trunk\lucene\tools\custom-tasks.xml:22: Problem: failed to create task or type licenses Cause: The name is undefined. Action: Check the spelling. Action: Check that any custom tasks/types have been declared. Action: Check that any <presetdef>/<macrodef> declarations have taken place.
          Hide
          Steve Rowe added a comment -

          Timings for me under Ant 1.7.1:

          validate:
               [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\lucene
           [licenses] Scanned 7 JAR file(s) for licenses (in 0.79s.), 0 error(s).
          
          validate:
               [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\modules
           [licenses] Scanned 11 JAR file(s) for licenses (in 1.38s.), 0 error(s).
          
          [...]
          
          validate:
               [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\solr
          [...]
           [licenses] Scanned 73 JAR file(s) for licenses (in 1.37s.), 1 error(s).
          
          Show
          Steve Rowe added a comment - Timings for me under Ant 1.7.1: validate: [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\lucene [licenses] Scanned 7 JAR file(s) for licenses (in 0.79s.), 0 error(s). validate: [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\modules [licenses] Scanned 11 JAR file(s) for licenses (in 1.38s.), 0 error(s). [...] validate: [echo] License check under: C:\cygwin\home\s\svn\lucene\dev\trunk\solr [...] [licenses] Scanned 73 JAR file(s) for licenses (in 1.37s.), 1 error(s).
          Hide
          Dawid Weiss added a comment -

          As for jsonic-NOTICE.txt, I have that file on disk, but it's zero length (didn't know what to put in there, to be honest). Maybe it didn't make it to the patch for this reason? As for post.jar – this should probably be excluded and generated in solr (it's most likely ignored because I, again, don't have it in my checkout).

          Show
          Dawid Weiss added a comment - As for jsonic-NOTICE.txt, I have that file on disk, but it's zero length (didn't know what to put in there, to be honest). Maybe it didn't make it to the patch for this reason? As for post.jar – this should probably be excluded and generated in solr (it's most likely ignored because I, again, don't have it in my checkout).
          Hide
          Dawid Weiss added a comment -

          Ok, fixed jsonic-NOTICE.txt (added the project's URL there). I'll check what's with ANT 1.7 and non-topmost builds.

          Show
          Dawid Weiss added a comment - Ok, fixed jsonic-NOTICE.txt (added the project's URL there). I'll check what's with ANT 1.7 and non-topmost builds.
          Hide
          Dawid Weiss added a comment -

          Ehm. Weird, I used ANT 1.7.1 and everything works for me (including ant validate from subfolders). Can you do ant -verbose?

          Show
          Dawid Weiss added a comment - Ehm. Weird, I used ANT 1.7.1 and everything works for me (including ant validate from subfolders). Can you do ant -verbose?
          Hide
          Dawid Weiss added a comment -

          Oh, I see what's wrong now. import is before compilation and it fails on putting the project structure together. I knew having it at the top-level would be simpler...

          Ok, I'll see what I can do about it, but it'll have to wait until tomorrow.

          Show
          Dawid Weiss added a comment - Oh, I see what's wrong now. import is before compilation and it fails on putting the project structure together. I knew having it at the top-level would be simpler... Ok, I'll see what I can do about it, but it'll have to wait until tomorrow.
          Hide
          Dawid Weiss added a comment -

          This patch fixes the issue with failing definition by delaying taskdef until the macro is invoked. Piggybacking NOTICE updates for jsonic and other minor things.

          Show
          Dawid Weiss added a comment - This patch fixes the issue with failing definition by delaying taskdef until the macro is invoked. Piggybacking NOTICE updates for jsonic and other minor things.
          Hide
          Steve Rowe added a comment -

          This patch fixes the issue with failing definition by delaying taskdef until the macro is invoked. Piggybacking NOTICE updates for jsonic and other minor things.

          Works for me from lucene/ and solr/ (no more license problems!), but not from modules/.

          Show
          Steve Rowe added a comment - This patch fixes the issue with failing definition by delaying taskdef until the macro is invoked. Piggybacking NOTICE updates for jsonic and other minor things. Works for me from lucene/ and solr/ (no more license problems!), but not from modules/ .
          Hide
          Dawid Weiss added a comment -

          Excluded /dist/ from validation too. Seems to work for me just fine. I'm done for the day.

          Show
          Dawid Weiss added a comment - Excluded /dist/ from validation too. Seems to work for me just fine. I'm done for the day.
          Hide
          Dawid Weiss added a comment -

          I assumed modules doesn't need to be self-contained like Lucene or Solr. I can fix that by enforcing tools' compilation in that macro... I'll do that.

          This reminds me of the "infrastructure tools" problem we had in Carrot2. We finally decided to simply have them as a stand-alone project living within the same repository space, with a stored, versioned binary artefact updated when tools had to be updated (rarely). This does version a binary file but you don't need to worry about recompiling things over and over.

          Show
          Dawid Weiss added a comment - I assumed modules doesn't need to be self-contained like Lucene or Solr. I can fix that by enforcing tools' compilation in that macro... I'll do that. This reminds me of the "infrastructure tools" problem we had in Carrot2. We finally decided to simply have them as a stand-alone project living within the same repository space, with a stored, versioned binary artefact updated when tools had to be updated (rarely). This does version a binary file but you don't need to worry about recompiling things over and over.
          Hide
          Dawid Weiss added a comment -

          Reordered the order of calls (first submodules then macro) in modules/build.xml.

          Cleaned up empty validation targets that only made ant output harder to read.

          Show
          Dawid Weiss added a comment - Reordered the order of calls (first submodules then macro) in modules/build.xml. Cleaned up empty validation targets that only made ant output harder to read.
          Hide
          Uwe Schindler added a comment -

          Thanks Dawid, especially for the really simple "example task" (I see this as simple example Task). I would like to see all other calls to Java that launches external JVMs to be converted to ANT (e.g. the Analyzer tasks).

          With you simple check-legal task, this is easy to do, as you can simply copypaste most parts.

          Show
          Uwe Schindler added a comment - Thanks Dawid, especially for the really simple "example task" (I see this as simple example Task). I would like to see all other calls to Java that launches external JVMs to be converted to ANT (e.g. the Analyzer tasks). With you simple check-legal task, this is easy to do, as you can simply copypaste most parts.
          Hide
          Dawid Weiss added a comment -

          No problem, like I said – we have dealt with ANT quite a lot in the past, so this comes easily. Having a task is definitely a speedup over spawning a separate JVM. I also like some of the tools ANT makes available even if they may feel like a clumsy way of expressing simple things at times.

          Show
          Dawid Weiss added a comment - No problem, like I said – we have dealt with ANT quite a lot in the past, so this comes easily. Having a task is definitely a speedup over spawning a separate JVM. I also like some of the tools ANT makes available even if they may feel like a clumsy way of expressing simple things at times.
          Hide
          Dawid Weiss added a comment -

          Steve, will you double check again if everything works and commit this in? I didn't put an entry into CHANGES so you'd have to add it if it qualifies at all to be mentioned there.

          Show
          Dawid Weiss added a comment - Steve, will you double check again if everything works and commit this in? I didn't put an entry into CHANGES so you'd have to add it if it qualifies at all to be mentioned there.
          Hide
          Steve Rowe added a comment -

          I assumed modules doesn't need to be self-contained like Lucene or Solr. I can fix that by enforcing tools' compilation in that macro... I'll do that.

          Steve, will you double check again if everything works

          Yes, everything works for me now, thanks!

          and commit this in?

          Is there some reason why you can't do this? IMO committers should commit their own work.

          I didn't put an entry into CHANGES so you'd have to add it if it qualifies at all to be mentioned there.

          IMO this definitely warrants a CHANGES.txt entry.

          This should also be backported to branch_3x.

          This reminds me of the "infrastructure tools" problem we had in Carrot2. We finally decided to simply have them as a stand-alone project living within the same repository space, with a stored, versioned binary artefact updated when tools had to be updated (rarely). This does version a binary file but you don't need to worry about recompiling things over and over.

          +1 to do this for this and any other Lucene/Solr Ant tasks.

          Show
          Steve Rowe added a comment - I assumed modules doesn't need to be self-contained like Lucene or Solr. I can fix that by enforcing tools' compilation in that macro... I'll do that. Steve, will you double check again if everything works Yes, everything works for me now, thanks! and commit this in? Is there some reason why you can't do this? IMO committers should commit their own work. I didn't put an entry into CHANGES so you'd have to add it if it qualifies at all to be mentioned there. IMO this definitely warrants a CHANGES.txt entry. This should also be backported to branch_3x. This reminds me of the "infrastructure tools" problem we had in Carrot2. We finally decided to simply have them as a stand-alone project living within the same repository space, with a stored, versioned binary artefact updated when tools had to be updated (rarely). This does version a binary file but you don't need to worry about recompiling things over and over. +1 to do this for this and any other Lucene/Solr Ant tasks.
          Hide
          Dawid Weiss added a comment -

          Is there some reason why you can't do this? IMO committers should commit their own work.

          Sure I can, but you started the issue and you're still the assignee, I just chimed in at some point

          I'll commit this in. Don't know if I'll find the time to backport tonight so feel free to do it if you have spare cycles.

          Show
          Dawid Weiss added a comment - Is there some reason why you can't do this? IMO committers should commit their own work. Sure I can, but you started the issue and you're still the assignee, I just chimed in at some point I'll commit this in. Don't know if I'll find the time to backport tonight so feel free to do it if you have spare cycles.
          Hide
          Steve Rowe added a comment -

          OK - please assign to yourself.

          Show
          Steve Rowe added a comment - OK - please assign to yourself.
          Hide
          Steve Rowe added a comment -

          Hi Dawid,

          I noticed you put a CHANGES.txt entry under the 4.0 section, but since this will be backported, it should go under the 3.6 section. Also, you put your entry under "Optimizations", but I think it would better go under "Build".

          Show
          Steve Rowe added a comment - Hi Dawid, I noticed you put a CHANGES.txt entry under the 4.0 section, but since this will be backported, it should go under the 3.6 section. Also, you put your entry under "Optimizations", but I think it would better go under "Build".
          Hide
          Dawid Weiss added a comment -

          See, that's why I insisted you do the commit

          I'm used to a different CHANGES organization (changes always go to trunk, then put a divider once released) – I'll fix according to your suggestions, but I'll do it later in the evening.

          Show
          Dawid Weiss added a comment - See, that's why I insisted you do the commit I'm used to a different CHANGES organization (changes always go to trunk, then put a divider once released) – I'll fix according to your suggestions, but I'll do it later in the evening.
          Hide
          Dawid Weiss added a comment -

          Manually merged and corrected backport. Didn't test extensively, but ant validate from the top level, lucene/ and solr/ passes.

          Show
          Dawid Weiss added a comment - Manually merged and corrected backport. Didn't test extensively, but ant validate from the top level, lucene/ and solr/ passes.
          Hide
          Steve Rowe added a comment -

          Hi Dawid,

          I noticed that on branch_3x, there is no <macrodef> for license-check-macro - I guess you didn't finish the merge from trunk to branch_3x?

          Show
          Steve Rowe added a comment - Hi Dawid, I noticed that on branch_3x, there is no <macrodef> for license-check-macro - I guess you didn't finish the merge from trunk to branch_3x?
          Hide
          Dawid Weiss added a comment -

          Mhmm... this would be weird, wouldn't it? I'll check.

          Show
          Dawid Weiss added a comment - Mhmm... this would be weird, wouldn't it? I'll check.
          Hide
          Dawid Weiss added a comment -

          Mhmm... this would be weird, wouldn't it? I'll check.

          Show
          Dawid Weiss added a comment - Mhmm... this would be weird, wouldn't it? I'll check.
          Hide
          Dawid Weiss added a comment -

          It's under lucene/tools/custom-tasks.xml? It needs to be in a separate file because it's used in modules (in trunk) where common-build.xml doesn't apply (isn't imported).

          Show
          Dawid Weiss added a comment - It's under lucene/tools/custom-tasks.xml? It needs to be in a separate file because it's used in modules (in trunk) where common-build.xml doesn't apply (isn't imported).
          Hide
          Steve Rowe added a comment -

          Aha - sorry for the noise - I was restricting my search (in IntelliJ and using grep ... $(find ...) from the cmdline) to just *build.xml <facepalm>

          Show
          Steve Rowe added a comment - Aha - sorry for the noise - I was restricting my search (in IntelliJ and using grep ... $(find ...) from the cmdline) to just *build.xml <facepalm>
          Hide
          Steve Rowe added a comment -

          It needs to be in a separate file because it's used in modules (in trunk) where common-build.xml doesn't apply (isn't imported).

          FYI, trunk/modules/**/build.xml do import common-build.xml, because they import contrib/contrib-build.xml, which in turn imports common-build.xml.

          Show
          Steve Rowe added a comment - It needs to be in a separate file because it's used in modules (in trunk) where common-build.xml doesn't apply (isn't imported). FYI, trunk/modules/**/build.xml do import common-build.xml , because they import contrib/contrib-build.xml , which in turn imports common-build.xml .
          Hide
          Steve Rowe added a comment -

          Crap, my last comment is wrong too: trunk/modules/build.xml does not import lucene/contrib/contrib-build.xml at all, and this is where the license check stuff goes for modules/, so of course your explanation is exactly correct. <sigh>

          Show
          Steve Rowe added a comment - Crap, my last comment is wrong too: trunk/modules/build.xml does not import lucene/contrib/contrib-build.xml at all, and this is where the license check stuff goes for modules/ , so of course your explanation is exactly correct. <sigh>
          Hide
          Dawid Weiss added a comment -

          No worries, I'm glad you're double checking after me. Makes me confident nothing slipped through.

          Show
          Dawid Weiss added a comment - No worries, I'm glad you're double checking after me. Makes me confident nothing slipped through.
          Hide
          Yonik Seeley added a comment -

          I have a different view on this. Things like this (license checking) are typically integration tests. Having them per-module only complicates build files and is an unnecessary overhead for running normal tests (because dependencies change rarely).

          +1

          Having been bit by the changes in this issue dozens of times already, we shouldn't be doing these checks on a normal "ant test". Seems like it should be fine to let Jenkins test it.

          • SolrCloud demo instructions that have you make a copy of example it example2, etc.
          • mv build build.old so I could compare two runs
          • try out a new jar locally w/o dotting all the i's

          I've seen users report these errors on the mailing list too, and it's not apparent to them what the issue is.

          Show
          Yonik Seeley added a comment - I have a different view on this. Things like this (license checking) are typically integration tests. Having them per-module only complicates build files and is an unnecessary overhead for running normal tests (because dependencies change rarely). +1 Having been bit by the changes in this issue dozens of times already, we shouldn't be doing these checks on a normal "ant test". Seems like it should be fine to let Jenkins test it. SolrCloud demo instructions that have you make a copy of example it example2, etc. mv build build.old so I could compare two runs try out a new jar locally w/o dotting all the i's I've seen users report these errors on the mailing list too, and it's not apparent to them what the issue is.
          Hide
          Robert Muir added a comment -

          I agree too I think, its worse now that checking licenses means we have to resolve first,
          to ensure the jars actually exist. this adds overhead, maybe jenkins is good enough?
          it runs many times a day and we don't actually change jars that often: most of the time
          when developing we are just changing code...

          Show
          Robert Muir added a comment - I agree too I think, its worse now that checking licenses means we have to resolve first, to ensure the jars actually exist. this adds overhead, maybe jenkins is good enough? it runs many times a day and we don't actually change jars that often: most of the time when developing we are just changing code...
          Hide
          Dawid Weiss added a comment -

          I'm for pushing it to the top level. This will simplify handling of exceptional patterns and such too. Shouldn't be much of a problem to move it too.

          Show
          Dawid Weiss added a comment - I'm for pushing it to the top level. This will simplify handling of exceptional patterns and such too. Shouldn't be much of a problem to move it too.

            People

            • Assignee:
              Dawid Weiss
              Reporter:
              Steve Rowe
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development