Make corrections to the "arc amend" workflow in Mercurial repositories to be compatible with PHP 5+
ClosedPublic
Actions

Authored by cspeckmim on Sep 16 2021, 6:54 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T13665: The "arc amend" workflow does not work on Mercurial repositories

Commits

rARCa028291f8e5e: Make corrections to the "arc amend" workflow in Mercurial repositories to be…

Summary

Refs T13665

In the update to D21716 the str_starts_with function was added which is only available in PHP 8. This changes it to use strncmp() which is compatible back to PHP 4.

Additionally fixed running into an error when trying to run arc amend on a commit which already matches the content known in Phabricator. Mercurial throws an error when running amend without file changes and using the exact same commit message it already has.

Test Plan

I created a commit and revision then used hg amend -e to modify the commit message locally.
I used arc amend to update the commit message back to the appropriate message from Phabricator.
I ran arc amend again to verify that the command did not fail even though the commit message is unchanged.

I ran arc inspect --explore -- 'commit(674492bb460)' and verified it matched the appropriate commit, to verify the substring matching works properly.

Diff Detail

Repository

rARC Arcanist

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

cspeckmim created this revision.Sep 16 2021, 6:54 PM

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptSep 16 2021, 6:54 PM

Herald added a subscriber: epriestley. · View Herald Transcript

Harbormaster completed remote builds in B25567: Diff 51769.Sep 16 2021, 6:55 PM

cspeckmim requested review of this revision.Sep 16 2021, 6:55 PM

Switch to using strncmp()

Harbormaster completed remote builds in B25568: Diff 51770.Sep 16 2021, 6:58 PM

epriestley accepted this revision.Sep 17 2021, 2:20 AM

This revision is now accepted and ready to land.Sep 17 2021, 2:20 AM

Herald added a subscriber: Korvin. · View Herald TranscriptSep 17 2021, 2:20 AM

cspeckmim edited the summary of this revision. (Show Details)Sep 17 2021, 3:01 AM

cspeckmim edited the test plan for this revision. (Show Details)

cspeckmim retitled this revision from Make corrections to the "arc amend" workflow used with Mercurial repositories to Make corrections to the "arc amend" workflow in Mercurial repositories to be compatible with PHP 5+.

Closed by commit rARCa028291f8e5e: Make corrections to the "arc amend" workflow in Mercurial repositories to be… (authored by cspeckmim). · Explain WhySep 17 2021, 3:03 AM

This revision was automatically updated to reflect the committed changes.

cspeckmim added a commit: rARCa028291f8e5e: Make corrections to the "arc amend" workflow in Mercurial repositories to be….

The second baby has arrived so I have about 17 seconds per day to look at my computer nowadays, but I think it would also be reasonable to backfill str_starts_with() if you run into more of this -- the strncmp() syntax has always felt pretty hard to read to me. You can do that in PHP like this:

if (!function_exists('str_starts_with')) {
  function str_starts_with(...) {

  }
}

The second baby has arrived

I think it would also be reasonable to backfill str_starts_with() if you run into more of this -- the strncmp() syntax has always felt pretty hard to read to me.

I didn't realize functions could be conditionally defined like that - I played around with it testing different versions and it looks pretty straightforward. I'll keep that in mind for future changes.

cburroughs awarded a token.Oct 29 2021, 2:47 PM

cburroughs added a subscriber: cburroughs.