Add build config for Gson subset

[Marcono1234]

Purpose

Follow-up for #2946

Adds a build config which builds the Gson API subset, so that any changes which break this can be directly noticed.

Description

The Maven config is unfortunately a bit verbose because:

• it has to make sure only the explicitly listed source files are compiled
• it should not use the already compiled class files from target/classes

See also the comments in the changed code. Any hints on how this can be simplified are welcome.

As requested in #2946 (comment), the new GitHub workflow job will cause a workflow failure when building the Gson API subset fails.

[eamonnmcmanus]

Thanks for doing this, it is great!

Just a couple of things.

[eamonnmcmanus]

I did not find I needed to include JsonSerializer and JsonSerializationContext in the subset we're using internally at Google. Are they needed here for some reason?

[Marcono1234]

No, I just guessed the subset. I have also removed JsonDeserializer and JsonDeserializationContext now. I hope the list now matches what you are using.

[eamonnmcmanus]

Weird, I was sure I had commented with the exact list of source files that we're including internally, but I don't see my comment. Here it is again:

gson/src/main/java/com/google/gson/FormattingStyle.java
gson/src/main/java/com/google/gson/JsonArray.java
gson/src/main/java/com/google/gson/JsonElement.java
gson/src/main/java/com/google/gson/JsonIOException.java
gson/src/main/java/com/google/gson/JsonNull.java
gson/src/main/java/com/google/gson/JsonObject.java
gson/src/main/java/com/google/gson/JsonParseException.java
gson/src/main/java/com/google/gson/JsonParser.java
gson/src/main/java/com/google/gson/JsonPrimitive.java
gson/src/main/java/com/google/gson/JsonStreamParser.java
gson/src/main/java/com/google/gson/JsonSyntaxException.java
gson/src/main/java/com/google/gson/LongSerializationPolicy.java
gson/src/main/java/com/google/gson/Strictness.java
gson/src/main/java/com/google/gson/TypeAdapter.java
gson/src/main/java/com/google/gson/internal/JsonReaderInternalAccess.java
gson/src/main/java/com/google/gson/internal/LazilyParsedNumber.java
gson/src/main/java/com/google/gson/internal/LinkedTreeMap.java
gson/src/main/java/com/google/gson/internal/NonNullElementWrapperList.java
gson/src/main/java/com/google/gson/internal/NumberLimits.java
gson/src/main/java/com/google/gson/internal/Streams.java
gson/src/main/java/com/google/gson/internal/TroubleshootingGuide.java
gson/src/main/java/com/google/gson/internal/bind/JsonElementTypeAdapter.java
gson/src/main/java/com/google/gson/internal/bind/JsonTreeReader.java
gson/src/main/java/com/google/gson/internal/bind/JsonTreeWriter.java
gson/src/main/java/com/google/gson/package-info.java
gson/src/main/java/com/google/gson/stream/JsonReader.java
gson/src/main/java/com/google/gson/stream/JsonScope.java
gson/src/main/java/com/google/gson/stream/JsonToken.java
gson/src/main/java/com/google/gson/stream/JsonWriter.java
gson/src/main/java/com/google/gson/stream/MalformedJsonException.java

In particular, yes, it does include JsonParser.

[eamonnmcmanus]

Also, in the comment that I remember writing but don't see, I mentioned that I wasn't sure that LongSerializationPolicy belonged in the list, since I don't think you can use it without also using the Gson class.

[Marcono1234]

Have adjusted the list of classes. But LongSerializationPolicy indeed only seems to be relevant for Gson / GsonBuilder, so I have omitted it.

[eamonnmcmanus]

Would it be possible too to run SubsetTest against the copied subset of sources?

[Marcono1234]

Have removed this again because otherwise the test compilation cannot see these classes, and it looks like at least maven-compiler-plugin itself does not support extending the classpath? (could maybe be done through external plugins, but not sure if that is worth it)

I guess this is not an issue as long as you always run mvn clean ... to be safe, respectively don't cache the target/ dir in CI.

[Marcono1234]

Would it be possible too to run SubsetTest against the copied subset of sources?

Have adjusted the PR with one possible solution for this, and have included some of the other tests as well. I hope that is ok and useful; if not I can limit it to SubsetTest.

I also made some small changes because JsonParser is currently not part of the subset. Should it be part of the subset?

I am not that sure though how useful it is to run the tests for the subset, given that the regular Gson build runs these tests as well. I guess most issues would already pop up during the compilation of the subset.
These tests would only help if the subset referred to any classes through reflection only (something which would not be noticed as compilation error).

This subset build config feels rather brittle (I think it would also not notice non-existent <includes> or <testIncludes>), so maybe it would be good to use a less brittle setup in the long term.

[eamonnmcmanus]

Have adjusted the PR with one possible solution for this, and have included some of the other tests as well. I hope that is ok and useful; if not I can limit it to SubsetTest.

Yes, we also do that internally, with a fairly random selection of tests. The one that matters is SubsetTest, though, because we want to make sure that it builds and runs against the subset.

[Marcono1234]

Can't include JsonParserTest because its testReadWriteTwoObjects method uses Gson and refers to TestTypes, which uses a lot of Gson classes not included in the subset.

[eamonnmcmanus]

Great, thanks again! Is this ready to be merged?

[Marcono1234]

Yes I think so, if the list of classes and tests is fine for you like this.

As side note: For the <testIncludes> I think it might also compile (and eventually run) other test classes not explicitly listed, in case they are referenced by one of the classes listed as <testIncludes>¹. But that is probably not an issue? And if they referenced classes not part of the Gson subset, it seems compilation would still fail (as desired).

Footnotes

1. Otherwise the same approach as for the regular compilation might be needed, that is, copy the test classes to a separate directory and use that as <compileSourceRoots>. ↩

[eamonnmcmanus]

As side note: For the <testIncludes> I think it might also compile (and eventually run) other test classes not explicitly listed, in case they are referenced by one of the classes listed as <testIncludes>. But that is probably not an issue?

Yeah, that doesn't seem problematic to me.