Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Distribute binaries #161

Closed

Conversation

AidanDelaney
Copy link

@AidanDelaney AidanDelaney commented Aug 12, 2022

Support distribution of binaries in maven buildpack

closes #159

Summary

If no pom.xml is available the buildpack now provides "maven`

Adds a Command build plan metadata entry to request that a specific maven binary is distributed. A RunBuild build plan metadata entry allows subsequent buildpacks to omit project build stages. The changes allow the maven buildpack to be used in a "distribute only" mode.

Use Cases

Under discussion in #159

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@AidanDelaney AidanDelaney requested a review from a team August 12, 2022 08:53
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 12, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@AidanDelaney AidanDelaney marked this pull request as draft August 12, 2022 08:53
Add BP_MAVEN_COMMAND and RunBuild toggle in the build plan to control
whether to allow distribution-only.
Provide install functionality that can be re-used by Build.
If no maven project is found, allow subsequent buildpacks to request
that the maven buildpack makes either `mvn` or `mvnd` available.
If `mvn` or `mvnd` are installed for use by subsequent buildpacks,
then set MAVEN_OPTS to any configured maven settings.  This ensures
that subsequent buildpacks use the same `--settings` and
`-Dsettings.security`.
@AidanDelaney AidanDelaney marked this pull request as ready for review August 26, 2022 13:10
@dmikusa
Copy link
Contributor

dmikusa commented Sep 3, 2022

Adds a Command build plan metadata entry to request that a specific maven binary is distributed. A RunBuild build plan metadata entry allows subsequent buildpacks to omit project build stages. The changes allow the maven buildpack to be used in a "distribute only" mode.

I'm not sure I understand the need for the metadata entries. Can you expand on the high level design and your requirements?

  1. There is a buildplan entry for Maven (the binary). The buildpack provides and requires that if the pom.xml file exists. That seems reasonable to me.
  2. There is a buildplan entry for PlanEntryJVMApplicationPackage. Presently, this doesn't matter because detect won't pass unless something else requires it and so the buildpack itself doesn't need to check for this. However, if we're going to expand the buildpack to run and only provide binaries, then we'll need to check for the presence of this buildplan entry. If it's present, that can be a trigger for the build phase. An example is executable-jar, it will require PlanEntryJVMApplicationPackage if it needs source code to be compiled into a JAR. If it doesn't, because it already has a JAR file then this entry isn't added and Maven won't run.

If it helps we can try to graph out the buildplan entries and provide some example flows. This is probably the most complicated part of the Java buildpack buildplan, because it covers both source and pre-compiled asset builds. We have to be very careful making changes with this logic.

@AidanDelaney
Copy link
Author

At a high-level I want a buildpack to provide mvn or mvnd to subsequent buildpacks at build time. In addition, I do not want to run a build as the source project requiring maven may not be a JVM project. Subsequent buildpacks may then use mvn to perform operations. The concrete requirement in #159 is to use the maven-copy-dependencies plugin to copy dependencies.

I introduced a Command into the build plan metadata to allow a buildpack to request the maven buildpack to make either mvn or mvnd available as a build layer. The RunBuild build plan metadata allows subsequent buildpacks to disable running of the build logic.

Taking the README.md as a description of the logic, then I'm extending the logic as follows:

The buildpack will do the following:

  • Requests that a JDK be installed
  • Links the ~/.m2 to a layer for caching
  • If mvn or mvnd are requested for distribution then provide a build layer containing mvn or mvnd
  • if a source build is requested then
    • If <APPLICATION_ROOT>/mvnw exists
      • Runs <APPLICATION_ROOT>/mvnw -Dmaven.test.skip=true --no-transfer-progress package to build the application
    • If <APPLICATION_ROOT>/mvnw does not exist
      • Contributes Maven to a layer with all commands on $PATH
      • Runs <MAVEN_ROOT>/bin/mvn -Dmaven.test.skip=true --no-transfer-progress package to build the application
      • Caches $BP_MAVEN_BUILT_ARTIFACT to a layer
    • Removes the source code in <APPLICATION_ROOT>
    • If $BP_MAVEN_BUILT_ARTIFACT matched a single file
      • Restores $BP_MAVEN_BUILT_ARTIFACT from the layer, expands the single file to <APPLICATION_ROOT>
    • If $BP_MAVEN_BUILT_ARTIFACT matched a directory or multiple files
      • Restores the files matched by $BP_MAVEN_BUILT_ARTIFACT to <APPLICATION_ROOT>

I can see that the resolution of RunBuild in the current PR is incorrect. It should only be set to false only if there is a single build plan entry that requests RunBuild: false. Where other build plan entries exist for maven or jvm-application-package then RunBuild should be resolved to true.

Hopefully I've provided more clarity around the intention of providing a distribution of mvn or mvnd. The core idea is that there are situations where mvn is required, but a source build using mvn is not required.

@dmikusa
Copy link
Contributor

dmikusa commented Sep 11, 2022

Sorry for the delay on this. It is on my radar, I'm just trying to make some time to dig into it more. Thanks

@dmikusa dmikusa added semver:minor A change requiring a minor version bump type:enhancement A general enhancement labels Sep 12, 2022
@dmikusa
Copy link
Contributor

dmikusa commented Sep 15, 2022

I have been looking at this and working on some code changes for a few different scenarios.

I'd rather avoid having the command and runbuild metadata if we can help it. I think we can meet your requirements and not require that, but I'm still in the progress of testing build plans.

This is taking a little longer as I think it's necessitating some code clean up as well. The logic about picking which maven to use needs to be abstracted out.

Lastly, it is a little ambitious but Id like to add in support for the opposite case as well, ie just build with no install of Maven.

@AidanDelaney
Copy link
Author

Closing as #185 is a cleaner approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support distribution of binaries
2 participants