Skip to content

Commit a8e19e2

Browse files
stereotype441Commit Queue
authored andcommitted
Fix PRESUBMIT.py issue with manual presubmit invocations
I've been experimenting with using the Jj source control tool, and since it's not natively supported by `depot_tools`, this means I have to manually invoke `git cl presubmit`. `git cl presubmit` accepts a single argument which is the name of the upstream branch. The way this is intended to work is that the upstream branch name gets passed into the `PRESUBMIT.py` scripts, and they use it rather than making assumptions about what the upstream branch is. Prior to this change, our presubmit scripts for _fe_analyzer_shared, front_end, frontend_server, and kernel were ignoring the upstream branch and instead using git's `@{u}` shorthand (see https://git-scm.com/docs/git-rev-parse). This caused them to behave strangely when no upstream branch is set, which sometimes happens when running `git cl presubmit` manually. This change avoids the strange behavior by getting the upstream branch from the input to `PRESUBMIT.py`, as intended. Change-Id: I6a6a696423221d7b945b083fd72585f1f5a7e312 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447626 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent cc61a43 commit a8e19e2

File tree

6 files changed

+30
-10
lines changed

6 files changed

+30
-10
lines changed

pkg/_fe_analyzer_shared/PRESUBMIT.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ def runSmokeTest(input_api, output_api):
4848
print('WARNING: CFE et al presubmit_helper not found: %s' % test_helper)
4949
return []
5050

51-
args = [dart, test_helper, input_api.PresubmitLocalPath()]
51+
args = [
52+
dart, test_helper,
53+
input_api.PresubmitLocalPath(),
54+
input_api.change.UpstreamBranch()
55+
]
5256
process = subprocess.Popen(args,
5357
stdout=subprocess.PIPE,
5458
stdin=subprocess.PIPE)

pkg/front_end/PRESUBMIT.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ def runSmokeTest(input_api, output_api):
4848
print('WARNING: CFE et al presubmit_helper not found: %s' % test_helper)
4949
return []
5050

51-
args = [dart, test_helper, input_api.PresubmitLocalPath()]
51+
args = [
52+
dart, test_helper,
53+
input_api.PresubmitLocalPath(),
54+
input_api.change.UpstreamBranch()
55+
]
5256
process = subprocess.Popen(args,
5357
stdout=subprocess.PIPE,
5458
stdin=subprocess.PIPE)

pkg/front_end/presubmit_helper.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@ import 'test/utils/io_utils.dart';
1313
Future<void> main(List<String> args) async {
1414
Directory.current = Directory.fromUri(_repoDir);
1515
Stopwatch stopwatch = new Stopwatch()..start();
16-
// Expect something like /full/path/to/sdk/pkg/some_dir/whatever/else
17-
if (args.length != 1) throw "Need exactly one argument.";
16+
// Expect something like:
17+
// /full/path/to/sdk/pkg/some_dir/whatever/else upstreamBranch
18+
if (args.length != 2) throw "Need exactly two arguments.";
1819

19-
final List<String> changedFiles = getChangedFiles(collectUncommitted: false);
20+
String upstreamBranch = args[1];
21+
final List<String> changedFiles = getChangedFiles(
22+
collectUncommitted: false, upstreamBranch: upstreamBranch);
2023
String callerPath = args[0].replaceAll("\\", "/");
2124
if (!_shouldRun(changedFiles, callerPath)) {
2225
return;
@@ -291,7 +294,8 @@ Future<void> _executePendingWorkItems(List<Work> workItems) async {
291294
/// Queries git about changes against upstream, or origin/main if no upstream is
292295
/// set. This is similar (but different), I believe, to what
293296
/// `git cl presubmit` does.
294-
List<String> getChangedFiles({required bool collectUncommitted}) {
297+
List<String> getChangedFiles(
298+
{required bool collectUncommitted, required String upstreamBranch}) {
295299
Set<String> paths = {};
296300
void collectChanges(ProcessResult processResult) {
297301
for (String line in processResult.stdout.toString().split("\n")) {
@@ -311,7 +315,7 @@ List<String> getChangedFiles({required bool collectUncommitted}) {
311315
"diff",
312316
"--name-status",
313317
"--no-renames",
314-
"@{u}...HEAD",
318+
"$upstreamBranch...HEAD",
315319
],
316320
runInShell: true);
317321
if (result.exitCode != 0) {

pkg/front_end/tool/format_cl.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import '../test/utils/io_utils.dart';
1212
Future<void> main() async {
1313
Uri executable = getDartExecutable();
1414
final List<String> allChangedFiles =
15-
getChangedFiles(collectUncommitted: true);
15+
getChangedFiles(collectUncommitted: true, upstreamBranch: '@{u}');
1616
if (allChangedFiles.isEmpty) {
1717
print("No changes in CL.");
1818
return;

pkg/frontend_server/PRESUBMIT.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ def runSmokeTest(input_api, output_api):
4848
print('WARNING: CFE et al presubmit_helper not found: %s' % test_helper)
4949
return []
5050

51-
args = [dart, test_helper, input_api.PresubmitLocalPath()]
51+
args = [
52+
dart, test_helper,
53+
input_api.PresubmitLocalPath(),
54+
input_api.change.UpstreamBranch()
55+
]
5256
process = subprocess.Popen(args,
5357
stdout=subprocess.PIPE,
5458
stdin=subprocess.PIPE)

pkg/kernel/PRESUBMIT.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ def runSmokeTest(input_api, output_api):
4848
print('WARNING: CFE et al presubmit_helper not found: %s' % test_helper)
4949
return []
5050

51-
args = [dart, test_helper, input_api.PresubmitLocalPath()]
51+
args = [
52+
dart, test_helper,
53+
input_api.PresubmitLocalPath(),
54+
input_api.change.UpstreamBranch()
55+
]
5256
process = subprocess.Popen(args,
5357
stdout=subprocess.PIPE,
5458
stdin=subprocess.PIPE)

0 commit comments

Comments
 (0)