git-cinnabar icon indicating copy to clipboard operation
git-cinnabar copied to clipboard

git push crashes in git-remote-hg for some revisions.

Open emilio opened this issue 3 years ago • 10 comments

$ ./mach try again                                                                                                                                                                                                                         
estimates: Runs 329 tasks (304 selected, 25 dependencies)
estimates: Total task duration 5 days, 3:05:14
estimates: In the top 13% of durations
estimates: Should take about 1:26:13 (Finished around 2022-10-04 17:50)
Bundling 1 changesetsfatal: called `Option::unwrap()` on a `None` value
   0:     0x55cf3a24e8a3 - backtrace::capture::Backtrace::new::hb013f7276af64f4f
   1:     0x55cf3a149166 - git_cinnabar::cinnabar_main::{{closure}}::h7bd0d865583b65f2
   2:     0x55cf3a3b1daa - std::panicking::rust_panic_with_hook::hf26e9d4f97b40096
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:702:17
   3:     0x55cf3a3b1ba9 - std::panicking::begin_panic_handler::{{closure}}::hfab912107608087a
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:586:13
   4:     0x55cf3a3b0534 - std::sys_common::backtrace::__rust_end_short_backtrace::h434b685ce8d9965b
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/sys_common/backtrace.rs:138:18
   5:     0x55cf3a3b1919 - rust_begin_unwind
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:584:5
   6:     0x55cf3a04b913 - core::panicking::panic_fmt::ha6dc7f2ab2479463
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:142:14
   7:     0x55cf3a04b7dd - core::panicking::panic::hb3ad04c589a0e3c8
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:48:5
   8:     0x55cf3a04c69a - <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::next::h22b90fc5ba6a6728
   9:     0x55cf3a137ee0 - git_cinnabar::remote_helper_push::{{closure}}::hb76f65d48be0aa77
  10:     0x55cf3a13c2f1 - git_cinnabar::git_remote_hg::hb4cca5379331d269
  11:     0x55cf3a12d8c6 - git_cinnabar::git_cinnabar::h28e84be448d41baa
  12:     0x55cf3a148de9 - cinnabar_main
  13:     0x55cf3a12ee4f - git_cinnabar::main::hd1c9008dea96ab12
  14:     0x55cf3a0553c3 - std::sys_common::backtrace::__rust_begin_short_backtrace::h330494a39ec4a538
  15:     0x55cf3a072309 - std::rt::lang_start::{{closure}}::h0045a313ba50b05c
  16:     0x55cf3a3a690e - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hcdfee62722e5e4b8
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:280:13
                           std::panicking::try::do_call::h84ca51609826746f
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:492:40
                           std::panicking::try::hd58075e533b8e0cb
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:456:19
                           std::panic::catch_unwind::h1ebac24d83cb6ce2
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panic.rs:137:14
                           std::rt::lang_start_internal::{{closure}}::h0145388a1edd1640
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/rt.rs:128:48
                           std::panicking::try::do_call::h7630182e534a0a32
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:492:40
                           std::panicking::try::h05b6544f0c6331dc
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:456:19
                           std::panic::catch_unwind::h77b2ba8fd3309f34
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panic.rs:137:14
                           std::rt::lang_start_internal::h6612c8a7a6861b8b
                               at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/rt.rs:128:20
  17:     0x55cf3a1613b2 - main

error: git-remote-hg died of signal 6
error: failed to push some refs to 'hg::ssh://hg.mozilla.org/try'
Error running mach:

    ['try', 'again']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file try| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

subprocess.CalledProcessError: Command '('/usr/bin/git', '-c', 'cinnabar.check=traceback', 'push', 'hg::ssh://hg.mozilla.org/try', '+HEAD:refs/heads/branches/default/tip')' returned non-zero exit status 1.

  File "/home/emilio/src/moz/gecko-7/tools/tryselect/mach_commands.py", line 391, in try_again
    return run(command_context, **kwargs)
  File "/home/emilio/src/moz/gecko-7/tools/tryselect/mach_commands.py", line 201, in run
    return mod.run(**kwargs)
  File "/home/emilio/src/moz/gecko-7/tools/tryselect/selectors/again.py", line 149, in run
    return push_to_try(
  File "/home/emilio/src/moz/gecko-7/tools/tryselect/push.py", line 226, in push_to_try
    vcs.push_to_try(commit_message)
  File "/home/emilio/src/moz/gecko-7/python/mozversioncontrol/mozversioncontrol/__init__.py", line 671, in push_to_try
    subprocess.check_call(
  File "/usr/lib/python3.10/subprocess.py", line 369, in check_call
    raise CalledProcessError(retcode, cmd)

emilio avatar Oct 04 '22 14:10 emilio

Hmm, after re-fetching and rebasing on top of central, it went away :/

$ g fetch mozilla                                                                                                                                                                                                                          
From hg::https://hg.mozilla.org/mozilla-unified
 - [deleted]                     (none)     -> mozilla/branches/default/b379b56bf47cdfb9149b134dd1f9cd4850d72cfc
Reading 29 changesets
Reading and importing 29 manifests
Reading and importing 99 revisions of 94 files
Importing 29 changesets
Checking 94 imported file root and head revisions
   1517c4c922f6b..a15bc5c2b86fa  bookmarks/autoland                                        -> mozilla/bookmarks/autoland
   c35054ab141b4..99d699a1dac13  bookmarks/beta                                            -> mozilla/bookmarks/beta
   1f5816249c4df..8772c3a3f24c6  bookmarks/central                                         -> mozilla/bookmarks/central
 * [new branch]                  branches/default/e4c79d4b676cca6003afdda79ef3c061f83352db -> mozilla/branches/default/e4c79d4b676cca6003afdda79ef3c061f83352db
   1517c4c922f6b..a15bc5c2b86fa  branches/default/tip                                      -> mozilla/branches/default/tip

emilio avatar Oct 04 '22 14:10 emilio

Can you check your reflog to find where you were at before and try again from there?

glandium avatar Oct 04 '22 20:10 glandium

This is on 1f5816249c4dfccedfe5f79682f1fc117d5a92df (hg: 0e384d802c84057a296e06a6b05f0326ed8c46e3) plus a few patches, attached below. patches.bundle.zip

emilio avatar Oct 07 '22 00:10 emilio

So what happens in your case is that the mercurial changeset git-cinnabar creates for 316f240032ff4832a24e3340b3e86af33d1f7030 is f0828943d68496487c9f8753b5f4632ab4697deb, and your metadata already knows that changeset as ae7d385ae0d20ede07b9c55909f1e86382094443. Those are two commits with the same content, same parent, same author date, same commit date. The only difference is that the newer one is gpg-signed. If you commit --amend, the problem goes away.

That's something cinnabar could handle better, but it's an uncommon edge case. It also only happens because the metadata for your try pushes is kept, which is also uncommon (it shouldn't happen unless you set cinnabar.data or remote.$remote.cinnabar-data to always ; or did you push this to another repo?).

Additionally, and related to this, your clone has diverged from a pure cinnabar clone because you've directly pushed gpg-signed commits to central or autoland or wherever, and the corresponding changesets don't carry the gpg signature, so when cloning from scratch, the commits are different. Your repo seems otherwise consistent, but I have a full fsck still running, which will tell more.

glandium avatar Oct 07 '22 02:10 glandium

(it shouldn't happen unless you set cinnabar.data or remote.$remote.cinnabar-data to always ; or did you push this to another repo?).

Oh, maybe you git cinnabar fetch'ed from try? That would do it...

glandium avatar Oct 07 '22 02:10 glandium

Yeah, I fetch from try relatively frequently (e.g., my patch fails on macOS, I fix it on my Mac, push to try again and update the patch stack on my Linux machine as well)

emilio avatar Oct 07 '22 08:10 emilio

Ok, so fetching from try storing metadata is #82. A workaround is to run git cinnabar rollback after git cinnabar fetch.

glandium avatar Oct 07 '22 08:10 glandium

The easiest in your case, though, is to disable gpg-signing on your cinnabar repo.

glandium avatar Oct 07 '22 08:10 glandium

Summarizing the things that should be fixed short-ish term for future-me's sake:

  • Detect and warn when something being pushed would not be fetched as the same commit
  • More gracefully handle the situation where pushing a commit generates a mercurial changeset that already exists locally but does not refer to the same commit.
  • Address #82.

glandium avatar Oct 07 '22 08:10 glandium

I see. Yeah I've pushed test fixes to autoland (with sheriff's permission), so that explains that divergence.

emilio avatar Oct 07 '22 08:10 emilio