Let ArcanistBundle print some Git patch headers
AcceptedPublic
Actions

Authored by avivey on Jul 26 2023, 04:36.

Details

Reviewers

ton
valerio.bozzolan

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15249: Generate [From, Subject, Date] fields in download raw .diff URL to support "git am" and friends

Summary

To allow git am to accept patches produced using this, let ArcanistBundle produce the right headers.

Ref T15249.

Test Plan

See D25359.

Diff Detail

Repository

rARC Arcanist

Branch

raw-diff-more-data

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 691
Build 691: arc lint + arc unit

Unit TestsFailed
View All

	Time	Test
	1,792 ms	ArcanistBundleTestCase::testGitRepository EXCEPTION (RuntimeException): strlen(): Passing null to parameter #1 ($string) of type string is deprecated #0 /home/avivey/devtools/arcanist/src/parser/ArcanistBundle.php(852): PhutilErrorHandler::handleError(8192, '...', '...', 852) #1 /home/avivey/devtools/arcanist/src/parser/ArcanistBundle.php(432): ArcanistBundle->buildBinaryChange(Object(ArcanistDiffChange), NULL)
	1 ms	ArcanistBaseCommitParserTestCase::testBasics 2 assertion(s) passed.
	0 ms	ArcanistBaseCommitParserTestCase::testHalt 1 assertion(s) passed.
	0 ms	ArcanistBaseCommitParserTestCase::testJump 1 assertion(s) passed.
	0 ms	ArcanistBaseCommitParserTestCase::testJumpReturn 1 assertion(s) passed.
		View Full Test Results (1 Failed · 74 Passed)

Event Timeline

avivey created this revision.Jul 26 2023, 04:36

Owners added a reviewer: O1: Blessed Committers.Jul 26 2023, 04:36

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJul 26 2023, 04:36

avivey requested review of this revision.Jul 26 2023, 04:36

Harbormaster completed remote builds in B691: Diff 1159.Jul 26 2023, 04:36

avivey added inline comments.Jul 26 2023, 04:37

src/workflow/ArcanistExportWorkflow.php
206	This is not related, but I was already here. `git config ...` adds a newline to the output.

avivey added a child revision: D25359: Generate more fields in Download Raw Diff.Jul 26 2023, 04:43

avivey edited the test plan for this revision. (Show Details)Jul 26 2023, 04:44

avivey mentioned this in T15249: Generate [From, Subject, Date] fields in download raw .diff URL to support "git am" and friends.Jul 26 2023, 04:47

a php 8 fix

Harbormaster completed remote builds in B693: Diff 1161.Jul 26 2023, 04:53

valerio.bozzolan added inline comments.Jul 26 2023, 06:41

src/workflow/ArcanistExportWorkflow.php
206	If the need is to remove a newline, note that trim() removes spaces

valerio.bozzolan awarded a token.Jul 26 2023, 06:41

avivey added inline comments.Jul 26 2023, 06:44

src/workflow/ArcanistExportWorkflow.php
206	close enough - email shouldn't have any whitespace in it, afaik.

valerio.bozzolan added inline comments.Jul 26 2023, 06:48

src/workflow/ArcanistExportWorkflow.php
206	Oh sorry - I'm wrong! trim() as default removes that since always http://web.archive.org/web/20010215013717/https://www.php.net/manual/en/function.trim.php And now I have to fix some other trims in other projects...

avivey mentioned this in D25359: Generate more fields in Download Raw Diff.Aug 2 2023, 19:46

Thanks again \o/

Note for other people: it is normal that sometime the user's email is not exposed even after this change: for example it happens when uploading a patch from the web interface (since Phorge is nice and respects users' privacy and we do not publish their emails as default).

The problem is, Phorge is nice and may not expose the email as default, but the user cannot add this information later. This is not nice, and this can indeed be discussed to further improvement. For example, allowing users to explicitly mark some personal email addresses as public, and so, we could start exposing them from Differential. Or, adding an explicit "email" field to Differential - but I don't know.

I'm just saying that this is nice as-is. Thanks

This revision is now accepted and ready to land.Aug 20 2023, 07:20