Page MenuHomePhabricator

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

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

Details

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

This revision is now accepted and ready to land.Sep 17 2021, 2:20 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+.

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

aw smile.png (100×100 px, 6 KB)
dancing duck.gif (280×498 px, 2 MB)

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.