Skip to content

Commit 7b0bee3

Browse files
Slightly relax Java Poison Pill on prerelease versions (-rc1, -dev, etc).
Before this, the poison pill intended to require exact match including suffix if there were suffixes on either gencode or runtime. After this change: - If gencode is suffixed, it must be exact match to runtime (including suffix). - If runtime is suffixed and gencode is not suffixed, the gencode must be a strictly lower major.minor.point (4.32.1-rc1 runtime must be on <= 4.32.0 gencode). We log a warning once if this allowed inequality state happens. This change also fixes a bug where a lower minor version was allowed if it did have the same suffix (4.33.0-rc1 runtime accidentally accepted 4.32.0-rc1 gencode while still rejecting 4.32.0). PiperOrigin-RevId: 813286207
1 parent fc0eda1 commit 7b0bee3

File tree

2 files changed

+154
-16
lines changed

2 files changed

+154
-16
lines changed

java/core/src/main/java/com/google/protobuf/RuntimeVersion.java

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ public enum RuntimeDomain {
4646
@SuppressWarnings("NonFinalStaticField")
4747
static int minorWarningLoggedCount = 0;
4848

49+
@SuppressWarnings("NonFinalStaticField")
50+
static boolean preleaseRuntimeWarningLogged = false;
51+
4952
private static final String VERSION_STRING = versionString(MAJOR, MINOR, PATCH, SUFFIX);
5053
private static final Logger logger = Logger.getLogger(RuntimeVersion.class.getName());
5154

@@ -117,6 +120,26 @@ private static void validateProtobufGencodeVersionImpl(
117120

118121
String gencodeVersionString = null;
119122

123+
if (!SUFFIX.isEmpty() && !preleaseRuntimeWarningLogged) {
124+
if (gencodeVersionString == null) {
125+
gencodeVersionString = versionString(major, minor, patch, suffix);
126+
}
127+
logger.warning(
128+
String.format(
129+
Locale.US,
130+
" Protobuf prelease version %s in use. This is not recommended for "
131+
+ "production use.\n"
132+
+ " You can ignore this message if you are deliberately testing a prerelease."
133+
+ " Otherwise you should switch to a non-prerelease Protobuf version.",
134+
VERSION_STRING));
135+
preleaseRuntimeWarningLogged = true;
136+
}
137+
138+
// Exact match is always good.
139+
if (major == MAJOR && minor == MINOR && patch == PATCH && suffix.equals(SUFFIX)) {
140+
return;
141+
}
142+
120143
// Check that runtime major version is the same as the gencode major version.
121144
if (major != MAJOR) {
122145
if (major == MAJOR - 1 && majorWarningLoggedCount < MAX_WARNING_COUNT) {
@@ -158,16 +181,44 @@ private static void validateProtobufGencodeVersionImpl(
158181
VERSION_STRING));
159182
}
160183

161-
// Check that runtime version suffix is the same as the gencode version suffix.
162-
if (!suffix.equals(SUFFIX)) {
184+
// If neither gencode or runtime has a suffix we're done.
185+
if (suffix.isEmpty() && SUFFIX.isEmpty()) {
186+
return;
187+
}
188+
189+
// If gencode has any suffix, we only support exact matching runtime including suffix. Exact
190+
// match including suffix was already checked above so if we get this far and the gencode has a
191+
// suffix it is a disallowed combination.
192+
if (!suffix.isEmpty()) {
193+
if (gencodeVersionString == null) {
194+
gencodeVersionString = versionString(major, minor, patch, suffix);
195+
}
196+
throw new ProtobufRuntimeVersionException(
197+
String.format(
198+
Locale.US,
199+
"Detected mismatched Protobuf Gencode/Runtime version suffixes when loading %s:"
200+
+ " gencode %s, runtime %s. Prerelease gencode must be used with the same"
201+
+ " runtime.",
202+
location,
203+
gencodeVersionString,
204+
VERSION_STRING));
205+
}
206+
207+
// Here we know the runtime version is suffixed and the gencode version is not. If the
208+
// major.minor.patch is exact match, its an illegal combination (eg 4.32.0-rc1 runtime with
209+
// 4.32.0 gencode). If major.minor.patch is not an exact match, then the gencode is a lower
210+
// version (e.g 4.32.0-rc1 runtime with 4.31.0 gencode) which is allowed.
211+
if (major == MAJOR && minor == MINOR && patch == PATCH) {
163212
if (gencodeVersionString == null) {
164213
gencodeVersionString = versionString(major, minor, patch, suffix);
165214
}
166215
throw new ProtobufRuntimeVersionException(
167216
String.format(
168217
Locale.US,
169218
"Detected mismatched Protobuf Gencode/Runtime version suffixes when loading %s:"
170-
+ " gencode %s, runtime %s. Version suffixes must be the same.",
219+
+ " gencode %s, runtime %s. Prelease runtimes must only be used with exact match"
220+
+ " gencode (including suffix) or non-prerelease gencode versions of a"
221+
+ " lower version.",
171222
location,
172223
gencodeVersionString,
173224
VERSION_STRING));

java/core/src/test/java/com/google/protobuf/RuntimeVersionTest.java

Lines changed: 100 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,18 +76,6 @@ public void versionValidation_versionNumbersAllTheSameAllowed() {
7676
"dummy");
7777
}
7878

79-
@Test
80-
public void versionValidation_newerRuntimeVersionAllowed() {
81-
int gencodeMinor = RuntimeVersion.MINOR - 1;
82-
RuntimeVersion.validateProtobufGencodeVersion(
83-
RuntimeVersion.DOMAIN,
84-
RuntimeVersion.MAJOR,
85-
gencodeMinor,
86-
RuntimeVersion.PATCH,
87-
RuntimeVersion.SUFFIX,
88-
"dummy");
89-
}
90-
9179
@Test
9280
public void versionValidation_olderRuntimeVersionDisallowed() {
9381
int gencodeMinor = RuntimeVersion.MINOR + 1;
@@ -146,8 +134,107 @@ public void versionValidation_differentVersionSuffixDisallowed() {
146134
+ " testing.Foo");
147135
}
148136

137+
@Test
138+
public void versionValidation_suffixedRuntime_logsWarning() {
139+
// We can only execute this test if the test runtime does have a suffix (which is nearly always
140+
// for our OSS continuous tests).
141+
if (RuntimeVersion.SUFFIX.isEmpty()) {
142+
return;
143+
}
144+
145+
// Suffixed runtimes only log the message once. To force the warning to be logged for
146+
// this test unrelated to test order, flip the bool back to false if it had been flipped by
147+
// another test to ensure the intended log can be observed.
148+
RuntimeVersion.preleaseRuntimeWarningLogged = false;
149+
150+
TestUtil.TestLogHandler logHandler = new TestUtil.TestLogHandler();
151+
Logger logger = Logger.getLogger(RuntimeVersion.class.getName());
152+
logger.addHandler(logHandler);
153+
RuntimeVersion.validateProtobufGencodeVersion(
154+
RuntimeVersion.DOMAIN,
155+
RuntimeVersion.MAJOR,
156+
RuntimeVersion.MINOR,
157+
RuntimeVersion.PATCH,
158+
RuntimeVersion.SUFFIX,
159+
"dummy");
160+
assertThat(logHandler.getStoredLogRecords()).hasSize(1);
161+
assertThat(logHandler.getStoredLogRecords().get(0).getMessage())
162+
.contains("You can ignore this message if you are deliberately testing a prerelease.");
163+
}
164+
165+
@Test
166+
public void versionValidation_suffixedRuntime_sameSuffixLowerMinorDisallowed() {
167+
// We can only execute this test if the test runtime does have a suffix (which is nearly always
168+
// for our OSS continuous tests).
169+
if (RuntimeVersion.SUFFIX.isEmpty()) {
170+
return;
171+
}
172+
RuntimeVersion.ProtobufRuntimeVersionException thrown =
173+
assertThrows(
174+
RuntimeVersion.ProtobufRuntimeVersionException.class,
175+
() ->
176+
RuntimeVersion.validateProtobufGencodeVersion(
177+
RuntimeVersion.DOMAIN,
178+
RuntimeVersion.MAJOR,
179+
RuntimeVersion.MINOR - 1,
180+
RuntimeVersion.PATCH,
181+
RuntimeVersion.SUFFIX,
182+
"testing.Foo"));
183+
assertThat(thrown)
184+
.hasMessageThat()
185+
.contains(
186+
"Detected mismatched Protobuf Gencode/Runtime version suffixes when loading"
187+
+ " testing.Foo");
188+
}
189+
190+
@Test
191+
public void versionValidation_suffixedRuntime_sameNumbersNoSuffixDisallowed() {
192+
// We can only execute this test if the test runtime does have a suffix (which is nearly always
193+
// for our OSS continuous tests).
194+
if (RuntimeVersion.SUFFIX.isEmpty()) {
195+
return;
196+
}
197+
RuntimeVersion.ProtobufRuntimeVersionException thrown =
198+
assertThrows(
199+
RuntimeVersion.ProtobufRuntimeVersionException.class,
200+
() ->
201+
RuntimeVersion.validateProtobufGencodeVersion(
202+
RuntimeVersion.DOMAIN,
203+
RuntimeVersion.MAJOR,
204+
RuntimeVersion.MINOR,
205+
RuntimeVersion.PATCH,
206+
"",
207+
"testing.Foo"));
208+
assertThat(thrown)
209+
.hasMessageThat()
210+
.contains(
211+
"Detected mismatched Protobuf Gencode/Runtime version suffixes when loading"
212+
+ " testing.Foo");
213+
}
214+
215+
@Test
216+
public void versionValidation_suffixedRuntime_allowedLowerVersionWarns() {
217+
// We can only execute this test if the runtime does have a suffix (which is nearly always for
218+
// our OSS continuous tests).
219+
if (RuntimeVersion.SUFFIX.isEmpty()) {
220+
return;
221+
}
222+
RuntimeVersion.validateProtobufGencodeVersion(
223+
RuntimeVersion.DOMAIN,
224+
RuntimeVersion.MAJOR,
225+
RuntimeVersion.MINOR - 1,
226+
RuntimeVersion.PATCH,
227+
"",
228+
"testing.Foo");
229+
}
230+
149231
@Test
150232
public void versionValidation_gencodeOneMajorVersionOlderWarning() {
233+
// Hack: if this is a suffixed runtime it may log the prerelease warning here if this
234+
// is the first test to run. Force the bool to true to avoid the warning happening during
235+
// this test only if it was the first one run.
236+
RuntimeVersion.preleaseRuntimeWarningLogged = true;
237+
151238
TestUtil.TestLogHandler logHandler = new TestUtil.TestLogHandler();
152239
Logger logger = Logger.getLogger(RuntimeVersion.class.getName());
153240
logger.addHandler(logHandler);
@@ -156,7 +243,7 @@ public void versionValidation_gencodeOneMajorVersionOlderWarning() {
156243
RuntimeVersion.MAJOR - 1,
157244
RuntimeVersion.MINOR,
158245
RuntimeVersion.PATCH,
159-
RuntimeVersion.SUFFIX,
246+
"",
160247
"dummy");
161248
assertThat(logHandler.getStoredLogRecords()).hasSize(1);
162249
assertThat(logHandler.getStoredLogRecords().get(0).getMessage())

0 commit comments

Comments
 (0)