Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-1064

Unnecessary maven-remote-resources-plugin configuration

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.7.0
    • Component/s: build
    • Labels:
      None

      Description

      Just got pinged with a question from someone about something I also noticed during 1.6.0-rc1 testing:

      When building from a clean checkout (or deleting the top-level target/ directory), you'll see the calcite build trying to pull down the jars we're trying to build:

      mvn package
      [INFO] Scanning for projects...
      [INFO] ------------------------------------------------------------------------
      [INFO] Reactor Build Order:
      [INFO]
      [INFO] Calcite
      [INFO] Calcite Avatica
      [INFO] Calcite Avatica Server
      [INFO] Calcite Linq4j
      [INFO] Calcite Core
      [INFO] Calcite Examples
      [INFO] Calcite Example CSV
      [INFO] Calcite Example Function
      [INFO] Calcite MongoDB
      [INFO] Calcite Piglet
      [INFO] Calcite Plus
      [INFO] Calcite Spark
      [INFO] Calcite Splunk
      [INFO] Calcite Ubenchmark
      [INFO]
      [INFO] ------------------------------------------------------------------------
      [INFO] Building Calcite 1.7.0-SNAPSHOT
      [INFO] ------------------------------------------------------------------------
      [INFO]
      [INFO] --- maven-checkstyle-plugin:2.12.1:check (validate) @ calcite ---
      [INFO]
      [INFO] --- git-commit-id-plugin:2.1.9:revision (default) @ calcite ---
      [INFO]
      [INFO] --- maven-remote-resources-plugin:1.5:process (default) @ calcite ---
      [INFO]
      [INFO] --- maven-remote-resources-plugin:1.5:process (root-resources) @ calcite ---
      Downloading: https://repository.apache.org/content/repositories/snapshots/org/apache/calcite/calcite-avatica/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: http://conjars.org/repo/org/apache/calcite/calcite-avatica/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: http://repository.apache.org/snapshots/org/apache/calcite/calcite-avatica/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: https://repository.apache.org/content/repositories/snapshots/org/apache/calcite/calcite-avatica/1.7.0-SNAPSHOT/calcite-avatica-1.7.0-SNAPSHOT.jar
      Downloading: http://conjars.org/repo/org/apache/calcite/calcite-avatica/1.7.0-SNAPSHOT/calcite-avatica-1.7.0-SNAPSHOT.jar
      Downloading: http://repository.apache.org/snapshots/org/apache/calcite/calcite-avatica/1.7.0-SNAPSHOT/calcite-avatica-1.7.0-SNAPSHOT.jar
      Downloading: http://repository.apache.org/snapshots/org/apache/calcite/calcite-linq4j/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: http://conjars.org/repo/org/apache/calcite/calcite-linq4j/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: https://repository.apache.org/content/repositories/snapshots/org/apache/calcite/calcite-linq4j/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: https://repository.apache.org/content/repositories/snapshots/org/apache/calcite/calcite-linq4j/1.7.0-SNAPSHOT/calcite-linq4j-1.7.0-SNAPSHOT.jar
      Downloading: http://conjars.org/repo/org/apache/calcite/calcite-linq4j/1.7.0-SNAPSHOT/calcite-linq4j-1.7.0-SNAPSHOT.jar
      Downloading: http://repository.apache.org/snapshots/org/apache/calcite/calcite-linq4j/1.7.0-SNAPSHOT/calcite-linq4j-1.7.0-SNAPSHOT.jar
      Downloading: https://repository.apache.org/content/repositories/snapshots/org/apache/calcite/calcite-core/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: http://conjars.org/repo/org/apache/calcite/calcite-core/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: http://repository.apache.org/snapshots/org/apache/calcite/calcite-core/1.7.0-SNAPSHOT/maven-metadata.xml
      Downloading: https://repository.apache.org/content/repositories/snapshots/org/apache/calcite/calcite-core/1.7.0-SNAPSHOT/calcite-core-1.7.0-SNAPSHOT.jar
      Downloading: http://conjars.org/repo/org/apache/calcite/calcite-core/1.7.0-SNAPSHOT/calcite-core-1.7.0-SNAPSHOT.jar
      Downloading: http://repository.apache.org/snapshots/org/apache/calcite/calcite-core/1.7.0-SNAPSHOT/calcite-core-1.7.0-SNAPSHOT.jar
      

      It looks like this was introduced in CALCITE-741. The intent of the change isn't necesarily wrong, but I believe this is just trying to work against Maven.

      I think the change to make for Maven to work as it wants is to make a new module dedicated to creating the assembly. That module will depend on the other modules which should properly create the transitive dependency list.

      It's also worth noting that the maven-remote-resources-plugin configuration littered everywhere is duplicative (this is already handled in the Apache parent pom).

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        Resolved in release 1.7.0 (2016-03-22).

        Show
        julianhyde Julian Hyde added a comment - Resolved in release 1.7.0 (2016-03-22).
        Hide
        elserj Josh Elser added a comment -

        Re-closing after applying the same to calcite-parent https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=48415a80b8f7f5763cbc3ba4e90c59e3dbeca389

        Ran through a mvn release:prepare, verified that correct DEPENDENCIES files exist in jars and the source release.

        Show
        elserj Josh Elser added a comment - Re-closing after applying the same to calcite-parent https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=48415a80b8f7f5763cbc3ba4e90c59e3dbeca389 Ran through a mvn release:prepare , verified that correct DEPENDENCIES files exist in jars and the source release.
        Hide
        elserj Josh Elser added a comment -

        Realized this morning that I didn't make the corresponding change to the calcite-parent pom. Oops.

        Show
        elserj Josh Elser added a comment - Realized this morning that I didn't make the corresponding change to the calcite-parent pom. Oops.
        Hide
        elserj Josh Elser added a comment -

        Fixed in https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=7901eda051dc756ba5a985d1cf2fed1942459281

        Jacques Nadeau, Julian Hyde, I ended up pushing this one because I didn't have any big change in the end. By moving the plugin into the apache-release profile, it runs after the modules have built and removes the need to try to pull those artifacts before the modules have built.

        Show
        elserj Josh Elser added a comment - Fixed in https://git1-us-west.apache.org/repos/asf?p=calcite.git;a=commit;h=7901eda051dc756ba5a985d1cf2fed1942459281 Jacques Nadeau , Julian Hyde , I ended up pushing this one because I didn't have any big change in the end. By moving the plugin into the apache-release profile, it runs after the modules have built and removes the need to try to pull those artifacts before the modules have built.
        Hide
        elserj Josh Elser added a comment -

        Alright, I think I have this working. Apologies, Julian and Jacques, I was down a rabbit hole myself

        I think the fix is as simple as moving the maven-remote-resources-plugin "root-resources" entry into the apache-release profile. When it's declared as a project plugin, it gets invoked at the generate-resources lifecycle phase which causes it to download the SNAPSHOT releases before it actually builds the project. However, with the maven-release-plugin, the profile only gets activated at the very end, when all of the modules were built and available via the Maven reactor.

        Need to investigate the output a bit more, but just wanted to report back while it was fresh.

        Show
        elserj Josh Elser added a comment - Alright, I think I have this working. Apologies, Julian and Jacques, I was down a rabbit hole myself I think the fix is as simple as moving the maven-remote-resources-plugin "root-resources" entry into the apache-release profile. When it's declared as a project plugin, it gets invoked at the generate-resources lifecycle phase which causes it to download the SNAPSHOT releases before it actually builds the project. However, with the maven-release-plugin, the profile only gets activated at the very end, when all of the modules were built and available via the Maven reactor. Need to investigate the output a bit more, but just wanted to report back while it was fresh.
        Hide
        elserj Josh Elser added a comment -

        Even if that means we have to re-open 741 and live with the empty DEPENDENCIES file. I see that as preferable to, say, creating a new module for the sole purpose of managing dependencies.

        Let me mock this up – it makes a lot of sense in my head. The root-pom for a project comes down to defining versions/configuration for dependencies/plugins/profiles, and then there is an module which generates a dedicated artifact (in our case, a source tarball). Hopefully an example will make this clearer

        Show
        elserj Josh Elser added a comment - Even if that means we have to re-open 741 and live with the empty DEPENDENCIES file. I see that as preferable to, say, creating a new module for the sole purpose of managing dependencies. Let me mock this up – it makes a lot of sense in my head. The root-pom for a project comes down to defining versions/configuration for dependencies/plugins/profiles, and then there is an module which generates a dedicated artifact (in our case, a source tarball). Hopefully an example will make this clearer
        Hide
        julianhyde Julian Hyde added a comment -

        If it fixes this problem, I don't mind backing out the change that fixed CALCITE-741. Even if that means we have to re-open 741 and live with the empty DEPENDENCIES file. I see that as preferable to, say, creating a new module for the sole purpose of managing dependencies.

        Show
        julianhyde Julian Hyde added a comment - If it fixes this problem, I don't mind backing out the change that fixed CALCITE-741 . Even if that means we have to re-open 741 and live with the empty DEPENDENCIES file. I see that as preferable to, say, creating a new module for the sole purpose of managing dependencies.
        Hide
        elserj Josh Elser added a comment -

        Typically, there is a dedicated "assemble" Maven module included in projects, which depends on all other modules for some project, whose express purpose is generating a tarball. I believe the problem here in Calcite is that the parent pom for the entire project serves this purpose which must run before any of the children modules is run.

        Anywho, I'm messing around with this to see if I can deal with it in Avatica. Will report back with a concrete change – that should help clarify things.

        Show
        elserj Josh Elser added a comment - Typically, there is a dedicated "assemble" Maven module included in projects, which depends on all other modules for some project, whose express purpose is generating a tarball. I believe the problem here in Calcite is that the parent pom for the entire project serves this purpose which must run before any of the children modules is run. Anywho, I'm messing around with this to see if I can deal with it in Avatica. Will report back with a concrete change – that should help clarify things.
        Hide
        jnadeau Jacques Nadeau added a comment -

        I'm not sure that what you suggest fixes the initial problem: that the source tarball had an empty DEPENDENCIES file. I agree that having a submodule and generating a dependencies file there would make it easy to create a file in that module. However, I'm not sure how you're proposing to incorporate this into the src build that we get with the apache release profile.

        If you have a better idea on how to solve that, that's great.

        Show
        jnadeau Jacques Nadeau added a comment - I'm not sure that what you suggest fixes the initial problem: that the source tarball had an empty DEPENDENCIES file. I agree that having a submodule and generating a dependencies file there would make it easy to create a file in that module. However, I'm not sure how you're proposing to incorporate this into the src build that we get with the apache release profile. If you have a better idea on how to solve that, that's great.

          People

          • Assignee:
            elserj Josh Elser
            Reporter:
            elserj Josh Elser
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development