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

Procedural Build Plugin #1567

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open

Conversation

skovati
Copy link
Contributor

@skovati skovati commented Sep 24, 2024

closes #1470

Description

Extract the build logic used to generate procedural scheduling (and soon to be constraint) jars into a separate gradle plugin, which can be applied to new projects. This drastically simplifies the new user experience (they don't have to copy paste some build logic) and allows us to "remotely" update build logic with bug fixes or new features, i.e., users can just update to v0.0.2 of our plugin to get e.g. new dynamic linking support.

Caution

The current implementation of this plugin requires anyone who wants to run gradle commands in the aerie repo to have a GITHUB_TOKEN env var populated with an API key. This is so that gradle can consume this plugin before "configuring" the project.

Summary of changes

  • Refactor procedural build logic into procedural/plugin/src/main/groovy/gov.nasa.ammos.aerie.procedural.plugin.gradle
  • Set up publishing to Github packages for this plugin
  • Configure base Aerie project to look for plugins in Github packages
  • Refactor e2e test procedure assets to use this new plugin

Verification

I've published many times to my aerie fork to test this plugin works as expected when published. Additionally, I've already published to aerie packages, so our e2e tests for this PR.

Documentation

We can update the new user guides for procedural scheduling / constraints with this plugin. Additionally we can streamline the build.gradle in the aerie mission model template repo.

Future work

It could also be useful to create an extension for this plugin, which would allow us to configure procedural builds something like the following, i.e. we can write our own configuration DSL.

plugins {
  id "gov.nasa.ammos.aerie.procedural.plugin"
}

procedures {
  sharedDependencies {
    implementation("some-big-jar-we-want-dynamically-linked")
  }
  schedulers = [
    "PlaceDSNEvents"
    "XbandWindows"
  ]
  constraints = [
    "XbandKeepOut"
  ]
}

It might also be fruitful to extend this plugin idea to mission models.

We can also drastically simplify our existing... 🤔

 ~ fd build.gradle --exec cat | wc -l
 2093

...two thousand lines of gradle build script by deduplicating shared logic (e.g. a convention script for all our java libraries that gets published and tested in the same way)

@skovati skovati added the build Changes that affect the build system or external dependencies label Sep 24, 2024
@skovati skovati self-assigned this Sep 24, 2024
@skovati skovati force-pushed the feature/procedural-build-plugin branch from bc3242f to 999aeb1 Compare September 24, 2024 23:48
@skovati skovati force-pushed the feature/procedural-build-plugin branch from 399bf91 to 1c8bef4 Compare October 9, 2024 15:42
@skovati skovati marked this pull request as ready for review October 9, 2024 15:55
@skovati skovati requested a review from a team as a code owner October 9, 2024 15:55
@skovati
Copy link
Contributor Author

skovati commented Oct 9, 2024

Question for @dandelany, should this plugin version track the Aerie version?

@skovati
Copy link
Contributor Author

skovati commented Oct 9, 2024

@dandelany another thought, we might need to expose ${{ github.actor }} and ${{ secrets.GITHUB_TOKEN }} as environment variables (for gradle to use) in our e2e tests workflows if these aren't already exposed as deployment level environment variables

@JoelCourtney
Copy link
Contributor

Can we add the procedural library dependencies in this plugin?

@skovati
Copy link
Contributor Author

skovati commented Oct 9, 2024

Hmm good question @JoelCourtney . We'd want to make sure end users can define which version of Aerie deps they want to grab. One option would be pinning this Gradle plugin to Aerie versions, so that applying version e.g. v2.22.0 of this gradle plugin would pull in v2.22.0 Aerie procedural deps 🤔

@JoelCourtney
Copy link
Contributor

I like that idea

@mattdailis
Copy link
Collaborator

Sounds reasonable to me - we'd just need to make sure we include the plugin in our release process. Would that be relatively straightforward?

@skovati
Copy link
Contributor Author

skovati commented Oct 14, 2024

I've taken a slightly different approach implementing this @JoelCourtney @mattdailis. The procedural plugin's version won't directly track Aerie version, since I wasn't able to propagate specific dependency versions to plugin consumers. Instead, the plugin will read the dependency version from properties.aerieVersion, which can be populated by our mission model template build script. See https://github.com/NASA-AMMOS/aerie-mission-model-template/blob/3cf4d104ba12caf4380341866f38abd94f63cea6/build.gradle#L27-L34

@JoelCourtney
Copy link
Contributor

I'm fine with using properties.aerieVersion for now, but in a future release I'd rather do it through a configuration in the procedures {} extension you mentioned in the description. Since we don't have that yet, I'm OK using a variable.

@JoelCourtney
Copy link
Contributor

I am totally baffled how the tests are passing... the plugin hasn't been published, yet both the e2e package and the procedural/examples package are fetching and applying them as if they are. It doesn't work on my machine, but it seems to work in gh workflows... ???

@skovati
Copy link
Contributor Author

skovati commented Oct 22, 2024

They actually are published already @JoelCourtney!

https://github.com/NASA-AMMOS/aerie/packages/2282306

I sort of accidentally did this, I didn't realize my test API token had write permissions to the aerie packages 🫣. As stated in the caution block, you'll need to expose GITHUB_TOKEN and GITHUB_ACTOR environment variables (populated correctly) to consume these plugins on your machine.

Copy link
Contributor

@JoelCourtney JoelCourtney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, well we'll need to fix the local plugin applications for e2e tests and procedural examples next time we update the plugin. Otherwise we won't be able to properly test plugin updates.

@skovati skovati added the DON'T MERGE Do Not Merge This Branch label Oct 22, 2024
@skovati
Copy link
Contributor Author

skovati commented Nov 5, 2024

Placing this on hold for now while we revisit the GITHUB_TOKEN requirement; ideally we could consume this plugin locally instead of requiring a build to already be published, but gradle requires that plugins are resolved at configuration time, and the plugin is built at build time in the gradle life cycle. Publishing and consuming in two steps was the only way I could "break" this cycle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Changes that affect the build system or external dependencies DON'T MERGE Do Not Merge This Branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Procedural jar gradle plugin
3 participants