Landing from a branch that directly tracks origin/master places one in
a detached HEAD state. Instead, examine if there is a local branch of
the name that we landed onto, that also tracks the upstream; if so,
switch to that.
Details
Details
Made a branch via git checkout -b testing origin/master
and tried to arc land
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- to-upstream/tracking-upstream
- Lint
Lint Passed - Unit
Test Failures - Build Status
Buildable 8644 Build 10010: arc lint + arc unit
Event Timeline
Comment Actions
Your test plan should also probably cover the old behavior (when no named branch exists) but I think the overall behavior is correct.
You should have commit access now. You can use "Land Revision" in the UI, or realign your origin to this install (if it's pointed at GitHub) and arc land.
| src/land/ArcanistGitLandEngine.php | ||
|---|---|---|
| 276 | This $path->isConnectedToRemote() call seems a little fragile: it may be testing the original $path, which was connected to the remote before we adjusted it. But I can't really come up with a way to rewrite it that is unambiguously more clear, and I believe the behavior and semantics are ultimately correct. | |
| src/land/ArcanistGitLandEngine.php | ||
|---|---|---|
| 276 | Yeah, I waffled on this a bit. If we didn't set it, then $path->getLength() is still 0. I can add that as an explicit check, though ->isConnectedToRemote() handles that case already. | |
Comment Actions
Yeah, I think it's fine as is. I can't imagine anyone's going to sneak a poorly-behaved cache into isConnectedToRemote() or anything.
I think we can reasonably get test coverage here too before any more major changes occur, we just don't have much support infrastructure for covering high-level workflows yet and I think it will take most of a day to build what I have in mind.
Comment Actions
Landing via the UI failed with https://secure.phabricator.com/drydock/operation/58/ ; I'll use the CLI.
Comment Actions
Oh, change didn't get pushed to the staging area:
https://secure.phabricator.com/drydock/lease/578/
I'd guess because you don't have a public key on this install yet.
Comment Actions
Hmm, still no dice. Is the output of arc diff showing that the push to the staging area succeeded?
(You can likely just land from the CLI whenever you're tired of fiddling with it, but also fine to keep clicking buttons if you want to get it to work.)
Comment Actions
Nope, no staging area:
$ arc diff HEAD~ Linting... LINT OKAY No lint problems. Running unit tests... PASS 16ms★ ArcanistLibraryTestCase::testLibraryMap PASS 9ms★ ArcanistLibraryTestCase::testEverythingImplemented PASS 36ms★ ArcanistLibraryTestCase::testMethodVisibility UNIT OKAY No unit test failures. SKIP STAGING Unable to determine repository for this change. Updated an existing Differential revision: Revision URI: https://secure.phabricator.com/D14420 Included changes: M src/land/ArcanistGitLandEngine.php
I'm going to go ahead and land this from the command line, unless this is interesting to debug from your point of view.
Comment Actions
Oh, we don't actually list the repository explicitly in .arcconfig so it won't be able to figure it out until you point your origin here so the origins match. Then you should get this from arc which:
$ arc which REPOSITORY To identify the repository associated with this working copy, arc followed this process: Configuration value "repository.callsign" is empty. This repository has no VCS UUID (this is normal for git/hg). The remote URI for this working copy is "ssh://dweller@secure.phabricator.com/diffusion/ARC/". Found a unique matching repository. This working copy is associated with the ARC repository. ...
...and it should push to staging.
Comment Actions
The problem was even though the upstream remote of the branch I was working on was in this arcanist, the remote named origin still pointed to our internal fork. Temporarily swapping the names of the upstreams around made it work.
Would you take a patch to use the new upstream path logic instead of hard-coding looking at "origin"?
Comment Actions
Would you take a patch to use the new upstream path logic instead of hard-coding looking at "origin"?
Sure -- or maybe we should look at the upstream first, then origin if that fails to identify a repository? Offhand, that seems always better and less likely to break anyone with a weird workflow.