Page MenuHomePhabricator

Fix arc land on odd/modern git-svn checkouts
ClosedPublic

Authored by asherkin on Sep 17 2018, 2:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 10:11 AM
Unknown Object (File)
Fri, Nov 22, 10:11 AM
Unknown Object (File)
Fri, Nov 22, 9:47 AM
Unknown Object (File)
Wed, Nov 13, 1:33 PM
Unknown Object (File)
Fri, Nov 8, 2:39 AM
Unknown Object (File)
Sun, Oct 27, 11:19 PM
Unknown Object (File)
Fri, Oct 25, 8:20 PM
Unknown Object (File)
Oct 24 2024, 2:59 PM
Subscribers

Details

Summary

The current code assumes git-svn is always working from a remote called
trunk, but if the repository is initialized without the -T option it
will instead be called git-svn, and if --prefix is used (which is
set by default to origin/ in Git 2+) the remote name will have the
specified prefix as well.

Instead, look at the fetch target refspec set in the git-svn config.

Fixes T13293.

Test Plan

arc land without errors (or manually creating a trunk branch) from a
checkout made with Git 2.18.0 (verified this manually on a non--T
checkout as well).

Diff Detail

Repository
rARC Arcanist
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Can you file a task for this that has step-by-step reproduction instructions (e.g., run this command, then run this command, here's where the thing breaks before this change)? I think this change is likely fine, I'm just not confident I'll be able to reproduce the issue when this refactors during T13098.

This revision now requires changes to proceed.May 14 2019, 2:24 PM
src/workflow/ArcanistLandWorkflow.php
319–320

Re-visiting this after the testing today, I'm not 100% confident this is actually going to behave correctly with multiple git-svn remotes or tracking branches, but neither are something I've ever used with it.

Thanks, T13293 makes this much clearer.

src/workflow/ArcanistLandWorkflow.php
319–320

I'm comfortable waiting for this to explode somewhere, multiple git-svn remotes seems wildly ambitious.

This revision is now accepted and ready to land.May 22 2019, 4:30 PM

Thanks for the review - I dropped in a comment referencing the task when committing so it's clear why this is here when the refactoring comes along 🙂