Skip to content

Commit 2f8f7e5

Browse files
whhonehsudhof
authored andcommitted
[copybara] allow rfc 822 format in custom-rev-id
The current regex enforces a suffix `REV_ID` for custom-rev-id so it can never meet RFC 822 format (no underscore) and does not work with git trailer (https://git-scm.com/docs/git-interpret-trailers). Loosen the restriction, and allow RFC 822 format. Change-Id: If2e136f5e5f65164cd55c3144247704b42727f16
1 parent 6f1af12 commit 2f8f7e5

File tree

2 files changed

+41
-3
lines changed

2 files changed

+41
-3
lines changed

java/com/google/copybara/Core.java

+7-3
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,12 @@
125125
@UsesFlags({GeneralOptions.class, DebugOptions.class})
126126
public class Core implements LabelsAwareModule, StarlarkValue {
127127

128-
// Restrict for label ids like 'BAZEL_REV_ID'. More strict than our current revId.
129-
private static final Pattern CUSTOM_REVID_FORMAT = Pattern.compile("[A-Z][A-Z_0-9]{1,30}_REV_ID");
128+
// Restrict for label ids like 'BAZEL_REV_ID' or 'Bazel-RevId'.
129+
//
130+
// It is suggested to meet the RFC 822 format (e.g., no underscore) so that it works with git
131+
// trailer (https://git-scm.com/docs/git-interpret-trailers).
132+
private static final Pattern CUSTOM_REVID_FORMAT =
133+
Pattern.compile("([A-Z][A-Z_0-9]{1,30}_REV_ID|[A-Z][a-zA-Z0-9-]{1,30}-RevId)");
130134
private static final String CHECK_LAST_REV_STATE = "check_last_rev_state";
131135
private final GeneralOptions generalOptions;
132136
private final WorkflowOptions workflowOptions;
@@ -2179,7 +2183,7 @@ private ImmutableMap<String, Object> getEndpoints(Structure endpoints)
21792183
// TODO(b/269526710): Enable more than one endpoint
21802184
check(fields.size() == 1 && Iterables.getOnlyElement(fields).equals("destination"),
21812185
"Temporarily core.action_migration only supports one endpoint called destination");
2182-
2186+
21832187
for (String fieldName : fields) {
21842188
Object epProvider = endpoints.getValue(fieldName);
21852189
check(epProvider instanceof EndpointProvider,

javatests/com/google/copybara/WorkflowTest.java

+34
Original file line numberDiff line numberDiff line change
@@ -5377,4 +5377,38 @@ public void testInvalidMigrationName() {
53775377
+ ")\n",
53785378
".*Migration name 'foo[|] bad;name' doesn't conform to expected pattern.*");
53795379
}
5380+
5381+
@Test
5382+
public void testCustomRevIdFormat_validUsingUnderscoreWithREV_ID() throws Exception {
5383+
origin.addSimpleChange(/*timestamp*/ 0);
5384+
extraWorkflowFields = ImmutableList.of("custom_rev_id = \"CUSTOM_REV_ID\"");
5385+
skylarkWorkflow("default", SQUASH).run(workdir, ImmutableList.of(HEAD));
5386+
5387+
ProcessedChange change = Iterables.getOnlyElement(destination.processed);
5388+
assertThat(change.getRevIdLabel()).isEqualTo("CUSTOM_REV_ID");
5389+
}
5390+
5391+
@Test(expected = ValidationException.class)
5392+
public void testCustomRevIdFormat_invalidUsingHyphenWithREV_ID() throws Exception {
5393+
origin.addSimpleChange(/*timestamp*/ 0);
5394+
extraWorkflowFields = ImmutableList.of("custom_rev_id = \"CUSTOM-REV_ID\"");
5395+
skylarkWorkflow("default", SQUASH).run(workdir, ImmutableList.of(HEAD));
5396+
}
5397+
5398+
@Test
5399+
public void testCustomRevIdFormat_validHyphenWithRevId() throws Exception {
5400+
origin.addSimpleChange(/*timestamp*/ 0);
5401+
extraWorkflowFields = ImmutableList.of("custom_rev_id = \"Custom-RevId\"");
5402+
skylarkWorkflow("default", SQUASH).run(workdir, ImmutableList.of(HEAD));
5403+
5404+
ProcessedChange change = Iterables.getOnlyElement(destination.processed);
5405+
assertThat(change.getRevIdLabel()).isEqualTo("Custom-RevId");
5406+
}
5407+
5408+
@Test(expected = ValidationException.class)
5409+
public void testCustomRevIdFormat_invalidUsingUnderscoreWithRevId() throws Exception {
5410+
origin.addSimpleChange(/*timestamp*/ 0);
5411+
extraWorkflowFields = ImmutableList.of("custom_rev_id = \"CUSTOM_RevId\"");
5412+
skylarkWorkflow("default", SQUASH).run(workdir, ImmutableList.of(HEAD));
5413+
}
53805414
}

0 commit comments

Comments
 (0)