[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: (fyi) PR#7324 updates how projects consume vendored guava

> Can testing of the artifact prior to release not be done against the staging repo?

I suspect it could be. The process for making these updates has not yet been defined, and I agree that having this intermediate state on every update is not ideal.

(also, I mistakenly wrote Guava in the above email; this fix was actually for gRPC)

Here's a proposal for a path forward:

1. Release a new vendored guava artifact
2. Switch all Beam projects to consume the vendored artifact from Maven Central
  2b. Add validation to ensure all projects reference guava consistently from Maven Central and not the build.

And then define the process for future vendoring updates:
3. Increment the vendored artifact version number, and make changes to the vendored source project
4. Push the new vendored artifact to a staging repository
5. In a separate commit, add the staging repository as a dependency source
6. Increment the vendored artifact dependency to the new version and update consuming code as necessary
7. Open a PR with all changes for validation / code review, but don't merge yet
8. Open a separate PR with just the vendored artifact changes (up to step 4)
9. Once reviewed/merged, kick off a release of the vendored artifact
10. Once released, remove staging repository for pending PR (undo step 5), and merge after reviewed.

This process would be more tedious / time-consuming, but less disrupting for other contributors.

On Wed, Dec 19, 2018 at 2:06 PM Thomas Weise <thw@xxxxxxxxxx> wrote:
Hi Scott,

Can testing of the artifact prior to release not be done against the staging repo?

It is unfortunate that we have to change the imports on every patch. Is it not sufficient to just update the artifact version?


On Wed, Dec 19, 2018 at 1:49 PM Scott Wegner <scott@xxxxxxxxxx> wrote:
Just a heads up: PR#7324 [1] just got merged which fixes an issue with our vendored guava artifact and JNI: see BEAM-6056 [2] for details. Important changes for you:

a. IntelliJ doesn't understand how our vendored guava is built and will give red squigglies. This is annoying but temporary while we release a new vendored artifact.
b. The guava vendoring prefix has changed. If you have work-in-progress, you'll hit this when you merge into master. Update any new guava imports to: org.apache.beam.vendor.grpc.v1p3p1.[..]

Read on for details..

The fix for BEAM-6056 included two changes:
1. Updated package prefix for vendored guava symbols: o.a.b.vendor.grpc.v1_13_1 -> o.a.b.vendor.grpc.v1p3p1
2. Move project dependencies to consume vendored guava from :beam-vendor-grpc-1_13_1 project rather than the published Maven artifact.

Consuming from the integrated project artifact (#2) was necessary in order to make the changes and validate them inline without needing to first publish a Maven release. However, this has some downsides: notably, IntelliJ struggles to understand shaded dependencies [3]. I believe we should move back to consuming pre-published vendor artifacts. The first step is to publish a new vendored beam-vendor-grpc-1_13_1 artifact; I can start that process.

Got feedback?