Solr
  1. Solr
  2. SOLR-8353

Support regex for skipping license checksums

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.5, 6.0
    • Component/s: Build
    • Labels:
      None

      Description

      It would be useful to be able to specify a regex for license checksums to skip in the build. Currently there are only two supported values:
      1) skipChecksum (i.e. regex=*)
      2) skipSnapshotsChecksum (i.e. regex=SNAPSHOT)

      A regex would be more flexible and allow testing the entire build while skipping a more limited set of checksums, e.g.:
      a) an individual library (i.e. regex=joda-time*)
      b) a suite of libraries (i.e. regex=hadoop*)

      We could make skipChecksum and skipSnapshotsChecksum continue to work for backwards compatbility reasons.

      1. SOLR-8353.patch
        6 kB
        Gregory Chanan
      2. SOLR-8353.patch
        4 kB
        Gregory Chanan
      3. SOLR-8353.patch
        4 kB
        Gregory Chanan

        Issue Links

          Activity

          Hide
          Mike Drob added a comment -

          Is this desirable? For the same reason that I am against having license headers easy to skip, I can see it easily leading us down a road to developer sloppiness if this is allowed.

          I guess this makes testing an upgrade to a dependency much easier, and there would still be Jenkins builds to enforce the rules.

          Show
          Mike Drob added a comment - Is this desirable? For the same reason that I am against having license headers easy to skip, I can see it easily leading us down a road to developer sloppiness if this is allowed. I guess this makes testing an upgrade to a dependency much easier, and there would still be Jenkins builds to enforce the rules.
          Hide
          Gregory Chanan added a comment -

          I thought about that, but given you can already skip all checksums, this can't be worse.

          Show
          Gregory Chanan added a comment - I thought about that, but given you can already skip all checksums, this can't be worse.
          Hide
          Gregory Chanan added a comment -

          Here's a patch that implements regex skiping while leaving skipChecksum and skipSnapshotsChecksum intact. I tested by changing the hadoop version and httpcore version (and passing -DskipChecksumRegex='hadoop.*') and verified that only httpcore complained.

          Show
          Gregory Chanan added a comment - Here's a patch that implements regex skiping while leaving skipChecksum and skipSnapshotsChecksum intact. I tested by changing the hadoop version and httpcore version (and passing -DskipChecksumRegex='hadoop.*') and verified that only httpcore complained.
          Hide
          Mark Miller added a comment -

          This is desirable for the same reason we added the other skip options - some of us have a fork of the build that has to deal with changing dependencies. Our build systems all get to work according to their needs - which is a good thing.

          None of these options change the experience for our Jenkins build machines, or a devs checkout.

          The downside doesn't really exist.

          Show
          Mark Miller added a comment - This is desirable for the same reason we added the other skip options - some of us have a fork of the build that has to deal with changing dependencies. Our build systems all get to work according to their needs - which is a good thing. None of these options change the experience for our Jenkins build machines, or a devs checkout. The downside doesn't really exist.
          Hide
          Mark Miller added a comment - - edited

          I can see it easily leading us down a road to developer sloppiness if this is allowed.

          One, it's not allowed. (eg it's just a local option - you can't make it a default or official jenkins override)

          Two, every developer can make all kinds of optional changes to the build - they can even override the version via sys prop! But they all happen to know that if they change the build params, they can't really expect the official build to pass anymore and if they forget, like other deviations, jenkins will admonish them.

          Show
          Mark Miller added a comment - - edited I can see it easily leading us down a road to developer sloppiness if this is allowed. One, it's not allowed. (eg it's just a local option - you can't make it a default or official jenkins override) Two, every developer can make all kinds of optional changes to the build - they can even override the version via sys prop! But they all happen to know that if they change the build params, they can't really expect the official build to pass anymore and if they forget, like other deviations, jenkins will admonish them.
          Hide
          Mark Miller added a comment -

          Patch looks good.

          A couple weeks ago I tried to do this via simple regex exclusion in the ant build file, but quickly learned it was going to take this route and punted.

          Show
          Mark Miller added a comment - Patch looks good. A couple weeks ago I tried to do this via simple regex exclusion in the ant build file, but quickly learned it was going to take this route and punted.
          Hide
          Gregory Chanan added a comment -

          Thanks for taking a look, Mark. New patch fixes one of the log messages.

          Show
          Gregory Chanan added a comment - Thanks for taking a look, Mark. New patch fixes one of the log messages.
          Hide
          Gregory Chanan added a comment -

          Hmm, when i don't specify a regex it gets passed to the ant task as the substiution, e.g. "$

          {skipChecksumRegex}

          "

          Show
          Gregory Chanan added a comment - Hmm, when i don't specify a regex it gets passed to the ant task as the substiution, e.g. "$ {skipChecksumRegex} "
          Hide
          Uwe Schindler added a comment -

          You have to define a default for the property in ANT.

          Show
          Uwe Schindler added a comment - You have to define a default for the property in ANT.
          Hide
          Gregory Chanan added a comment -

          New patch.

          Perhaps there is a better way to do this, but defining the property to "" under the check-licenses target in both lucene and solr seems to work.

          Also, changed skipChecksumRegex to skipRegexChecksum to keep the ordering consistent with skipSnapshotsChecksum.

          Show
          Gregory Chanan added a comment - New patch. Perhaps there is a better way to do this, but defining the property to "" under the check-licenses target in both lucene and solr seems to work. Also, changed skipChecksumRegex to skipRegexChecksum to keep the ordering consistent with skipSnapshotsChecksum.
          Hide
          ASF subversion and git services added a comment -

          Commit 1717870 from gchanan@apache.org in branch 'dev/trunk'
          [ https://svn.apache.org/r1717870 ]

          SOLR-8353: Support regex for skipping license checksums

          Show
          ASF subversion and git services added a comment - Commit 1717870 from gchanan@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1717870 ] SOLR-8353 : Support regex for skipping license checksums
          Hide
          ASF subversion and git services added a comment -

          Commit 1717872 from gchanan@apache.org in branch 'dev/branches/branch_5x'
          [ https://svn.apache.org/r1717872 ]

          SOLR-8353: Support regex for skipping license checksums

          Show
          ASF subversion and git services added a comment - Commit 1717872 from gchanan@apache.org in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1717872 ] SOLR-8353 : Support regex for skipping license checksums
          Hide
          Gregory Chanan added a comment -

          Committed to 5.5 and trunk. Thanks for taking a look Mark, Mike, Uwe.

          Show
          Gregory Chanan added a comment - Committed to 5.5 and trunk. Thanks for taking a look Mark, Mike, Uwe.

            People

            • Assignee:
              Gregory Chanan
              Reporter:
              Gregory Chanan
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development