Amphtml: Update Top-Level Travis Job with Integration to Maven Central/Sonatype

[GeorgeLuo]

We would like to publish a distribution of the Java amp validator jar to Maven Central Repository. As a standalone Travis job, these are the instructions to push to Maven Central:

branches:
  only:
  - master
  - stable
language: java
env:
  global:
  - secure: ...
  - secure: ...
  - secure: ...
  - secure: ...
jdk:
- openjdk8
before_install: 
install:
- mvn install -Dgpg.skip=true -P !build-extras -B -V
before_script: 
script: mvn test -P !build-extras -B
cache:
  directories:
  - "~/.m2/repository"
after_success:
- "./cd/before-deploy.sh"
- "./cd/deploy.sh"
after_failure:

To do this we need a set of Sonatype credentials (env.global.secure)
this link explains those fields:

https://github.com/osmlab/atlas/wiki/Gradle,-Travis-CI-and-Maven-Central

as:

global:
    - # GITHUB_SECRET_TOKEN
    - secure: "XXXXXX"
    - # SONATYPE_USERNAME
    - secure: "XXXXXX"
    - # SONATYPE_PASSWORD
    - secure: "XXXXXX"
    - # GPG_KEY_ID
    - secure: "XXXXXX"
    - # GPG_PASSPHRASE
    - secure: "XXXXXX"
    - GPG_KEY_LOCATION=".travis/secret.gpg"
    - ENCRYPTED_GPG_KEY_LOCATION=".travis/secret.gpg.enc"`

The trigger for this build job should be on changes to amphtml/validator/java. Additionally a job should be ran on a PR with changes to this directory to validate the maven tests pass (should not trigger a new publish operation to maven central).

Please let me know if any of these points need clarification or I have misunderstood the process.

@honeybadgerdontcare @rsimha

[rsimha]

Thanks for filing this issue, @GeorgeLuo. Here's an outline for how this functionality can be implemented.

we need a set of Sonatype credentials (env.global.secure)

Couple comments / questions on this:

• For push jobs, how long does deploy.sh take? What exactly does it do? (We'd want to run this only for new commits to amphtml's master branch, correct?)
• Do the maven tests need any of the credentials? (I assume not.)

The trigger for this build job should be on changes to amphtml/validator/java

This is easily done by adding a new build target called VALIDATOR_JAVA, like this:
https://github.com/ampproject/amphtml/blob/58a99467b92a6af72a6c47a2bf57fd5fdd2ad060/build-system/pr-check/build-targets.js#L200-L208

Additionally a job should be ran on a PR with changes to this directory to validate the maven tests pass

Invoking the tests can added as a new gulp wrapper task validator/ here:

https://github.com/ampproject/amphtml/blob/58a99467b92a6af72a6c47a2bf57fd5fdd2ad060/build-system/tasks/validator.js#L26-L38

And running the tests as part of CI can be done by calling the new gulp task here, when the VALIDATOR_JAVA build target is detected:

https://github.com/ampproject/amphtml/blob/58a99467b92a6af72a6c47a2bf57fd5fdd2ad060/build-system/pr-check/validator-tests.js#L48-L76

(should not trigger a new publish operation to maven central).

Correct, PR builds won't trigger a publish because they won't have access to secure credentials.

[rsimha]

@rcebulko this was the JAVA validator request I mentioned today. Would you be able to pick this up when time permits?

[GeorgeLuo]

I brought this problem space to someone who's done this before. I'll describe the process as I understand it from the beginning. Notably, Sonatype will not be involved in this flow and we would look to publish to bintray.

Add this to the amphtml/validator/java/pom.xml

<distributionManagement>
    <snapshotRepository>
        <id>bintray-amphtml-validator-repo</id>
        <url>http://dl.bintray.com/amphtml/maven</url>
    </snapshotRepository>
    <repository>
        <id>bintray-amphtml-validator-repo</id>
        <url>https://api.bintray.com/maven/amphtml/maven/tagchowder;publish=1</url>
    </repository>
</distributionManagement>

amphtml/validator/java/cd/deploy.sh (per my previous message)

#!/usr/bin/env bash
if [ "$TRAVIS_BRANCH" == 'master' ] && [ "$TRAVIS_PULL_REQUEST" == 'false' ]; then
    mvn deploy --settings cd/mvnsettings.xml
fi

amphtml/validator/java/cd/mvnsettings.xml (referenced from deploy.sh)

<settings>
    <servers>
        <server>
            <id>bintray-amphtml-validator-repo</id>
            <username>${env.BINTRAY_USER}</username>
            <password>${env.BINTRAY_API_KEY}</password>
        </server>
    </servers>
</settings>

The travis job shared before contains deprecated information, this is what it would look like with those removed. The Sonatype credentials are no longer needed.

branches:
  only:
  - master
  - stable
language: java
env:
  global:
jdk:
- openjdk8
before_install: 
install:
- mvn install -Dgpg.skip=true -P !build-extras -B -V
before_script: 
script: mvn test -P !build-extras -B
cache:
  directories:
  - "~/.m2/repository"
after_success:
- "./cd/deploy.sh"
after_failure: 

If all this works, we would end up with something that looks like this:

https://bintray.com/yahoo/maven/tagchowder

@rsimha let me know if this makes sense. The ask here would be a set of credentials to use for the the validator BINTRAY_USER and BINTRAY_API_KEY. So an account would need to be set up on bintray and travis provides those environment variables (let me know if this sounds wrong).

In regards to your questions:

deploy.sh does not time much time, that job essentially wraps a build and upload-like process. So won't go over 2 minutes. Ideally we want to run deploy only on changes to the validator/java directory.

no credentials are needed to run maven tests.

[rcebulko]

I think it would be fine to run this either on master push builds or on nightly release builds.

@GeorgeLuo what is the reason (if any) to opt for Bintray as opposed to Sonatype, packagecloud, or other similar binary hosting services?

Regardless of the service, I agree the first step will be getting together credentials, setting up the hosted repo, etc. From there, it looks like we can configure the maven settings XML config and test out deploy.sh manually a couple times. Then once that's in place, and we have an idea of how long that update takes and how frequently updates happen at all, we'll be better prepared to identify where/when Travis should execute the job.

As for running the maven tests (mvn test -P !build-extras -B), I'm a bit rusty on Java build/CI setups but my guess would be this can be done independently of pushing the release to Bintray, so we should be able to enable tests for the Java validator without much extra work. Does this sound right to you?

[rcebulko]

Also, what's the benefit of using mvn test !build-extras -B versus bazel run //:amphtml_validator_test? Do these have the same effect?

[GeorgeLuo]

hello @rcebulko

the reason I'm suggesting this now is because internally this is what we have done most recently. As I'm not familiar with the process, debugging will be more painless if we have a flow to compare with, documentation, experience, etc. That's the only reason.

In regards to your question about mvn test, yes it is my belief the before_script travis field would watch the response of the mvn test command.

In regards to the bazel question, yes, largely, it was believed we would be integrating with the bazel flow while the PR was being formulated. We completed it, and it not adopted, but essentially it is a port of the mvn instructions.

Do you have a suggestion for who should take ownership of the publication?

[rsimha]

Do you have a suggestion for who should take ownership of the publication?

I think it makes sense for @ampproject/wg-caching to own this part, with help from @ampproject/wg-infra for the actual integration.

[rcebulko]

@GeorgeLuo If I understand our Travis setup correctly, we aren't able to conditionally trigger jobs; we're only able to conditionally run commands once a job is running. Because Travis configs have to pick a language/environment for running tests, I think this may require running a Travis-Java job for every single build, which (after setup) checks if the validator files are touched at all. I'm not even sure if we'll be able to use our build-targets system since Travis will be in Java mode, not NodeJS mode. It's a bit odd to have a Java library in the same repo as a NodeJS framework. What was the motivation behind incorporating it here versus in a separate Java repository?

I'm also curious what you think the desired behavior should be in the event of a test failure. Are you suggesting it be a blocking check, which would force developers working on the validator to port changes to Java in order for a change to go through? Or would you suggest it be a non-blocking check just for detecting if something breaks?

[rsimha]

Some clarifications below:

we aren't able to conditionally trigger jobs; we're only able to conditionally run commands once a job is running.

This is correct. However, it's not a problem because we can use our existing build target system. See https://github.com/ampproject/amphtml/issues/27093#issuecomment-595463522 for details.

Because Travis configs have to pick a language/environment for running tests, I think this may require running a Travis-Java job for every single build, which (after setup) checks if the validator files are touched at all. I'm not even sure if we'll be able to use our build-targets system since Travis will be in Java mode, not NodeJS mode.

This isn't a problem. The environment setting for Travis VMs only determines the default tools installed. It doesn't prevent the execution of tasks using other languages / tools. We already do run Java tasks (like generating a custom compiler), so I'd imagine we can install the tools required by the Java validator by updating one or both of these sections:

https://github.com/ampproject/amphtml/blob/2ff07ebc4b10c6f90b3d28a9d029241c0617b8e9/.travis.yml#L13-L17
https://github.com/ampproject/amphtml/blob/2ff07ebc4b10c6f90b3d28a9d029241c0617b8e9/.travis.yml#L24-L31

It's a bit odd to have a Java library in the same repo as a NodeJS framework. What was the motivation behind incorporating it here versus in a separate Java repository?

Our repo contains several non-node components, so adding a Java library doesn't violate any precedent. I'll let @GeorgeLuo or @honeybadgerdontcare comment further on this.

[GeorgeLuo]

Right, in regards to the event of a test failure, I think this should be non-blocking for the pull request, but the job to publish the library should not fire.

In regards to this library's location in this repo, I could see pros and cons. A major reason in favor being validation is a foundational aspect in the ecosystem.

[estherkim]

@GeorgeLuo, can you explain why the Java AMP validator jar needs to be pushed to Maven Central Repository? It's unclear to me; maybe this will help us understand how this fits in amphtml's CI builds.

[GeorgeLuo]

@estherkim hi, our organization needs this library to validate amp4email content to determine the display behavior. Right now it exists in this repo and to use the library we would need to build the jar and store in an internal repo. From then on, we'd build the jar any time changes are made to the source code.

This flow would be simplified if there was a point that synced the code with a publicly available jar, this would be Maven Central. I believe this use case is shared by java developers who would prefer to include the library via pom rather than building the package.

Tying this into CI: a new library should be published when there are changes to the validator/java source code. There will be cases where the protoascii language set changes, which will break the maven build. This should not hinder a pull request in my opinion, as at that point, users can dependably use the maven jar available.

[honeybadgerdontcare]

Do you have a suggestion for who should take ownership of the publication?

I think it makes sense for @ampproject/wg-caching to own this part, with help from @ampproject/wg-infra for the actual integration.

@GeorgeLuo is a member of the Caching Working Group so they (we) can own it if @ampproject/wg-infra is okay with it, which it sounds like it is.

As to why this is here and not elsewhere. The Validator is an integral part of the AMP ecosystem and there are needs for it in a variety of languages to suit the ecosystem. We (WG: Caching) plan on additional languages besides Java to support that ecosystem. If this is problematic then lets decide that now and a way forward for validators. From WG: Caching perspective we'd prefer to keep this and related code in the same repo and keep it under ampproject/amphtml/validator/.

[rsimha]

Thanks for the additional context, @honeybadgerdontcare.

we'd prefer to keep this and related code in the same repo and keep it under ampproject/amphtml/validator/.

I fully agree with having the Java validator code live in amphtml. This will enable us to effectively test PRs like #27083, where runtime / extension code is changed side by side with validator protoascii.

(we) can own it if @ampproject/wg-infra is okay with it, which it sounds like it is.

As I see it, there are two parts to this discussion:

1. Running tests for the Java validator during Travis CI checks for amphtml
2. Deploying changes to Maven central when Java validator code is changed

Item 1 is in the process of being implemented in #27271. Today, amphtml Travis builds fail when the python-based validator tests fail, so it's reasonable for the same thing to happen with the Java validator tests.

For item 2, the open questions are whether amphtml Travis builds should be responsible for pushing to an external repository, and if so, whether they should fail when a push is unsuccessful.

I believe this is what @rcebulko and @estherkim were asking about as well. (Let me know if I missed something.)

[estherkim]

Thanks @rimsha, moving on to Item 2 -

Tying this into CI: a new library should be published when there are changes to the validator/java source code. There will be cases where the protoascii language set changes, which will break the maven build. This should not hinder a pull request in my opinion, as at that point, users can dependably use the maven jar available.

It sounds like Java validator changes should not be deployed until merged into master, at the earliest.

However, we don't use Travis to deploy amphtml and amphtml-validator. Not saying that it means we shouldn't start, but is the Java validator something that can be tied to amphtml-validator releases? Or perhaps its own release process to npm?

[GeorgeLuo]

Java validator changes should not be deployed until merged into master, at the earliest

yes this makes sense. The CI/CD I am used to runs everything besides publishing upon the PR being made. Once merged, the publish job is fired.

To your second point, I didn't know that. I don't have a strong view, I imagine there are best practices in place and it would make sense to decouple the publish step. I have been in situations where a publish is incomplete on a successful PR and triggering changes to re-publish is less than ideal.

[rsimha]

Update: With recent changes, we've arrived at a state where the java validator tests are not guaranteed to pass all the time on master.

With this in mind, here is what remains to be done:

• [ ] Fix #27939
• [ ] Find a means other than amphtml Travis builds to deploy the java validator (https://github.com/ampproject/amphtml/issues/27786#issuecomment-614264183)

Both these items are under the purview of @ampproject/wg-caching. Happy to help if you have any remaining questions.

[GeorgeLuo]

ack. thank you @rsimha. This works, actively working, this engine change is sizable and I need to be mindful of prioritizing it.

[GeorgeLuo]

Hello,

not sure if this topic needs a new ticket: @nhant01 and I are onboarding validator-java to the git-travis-sonatype ecosystem in order to publish the package.

We have filed this ticket with sonatype to onboard, please let us know if there are glaring issues with this strategy.

[GeorgeLuo]

@Gregable / @rsimha can you take a look at the last comment on the ticket above?