sort out last test-case to assure setting symbolic refs is safe (#450)

We don't want to create dangling references, ever, and believe that
it's not possible to do so with the code we have, as we will fall back
to pointing to the object itself otherwise.

git-repository/src/remote/connection/fetch/update_refs/mod.rs

@@ -173,7 +173,14 @@ pub(crate) fn update(
                                         .flatten()
                                 })
                             }) {
-                                Some(local_branch) => Target::Symbolic(local_branch), // TODO: even though it's on the list, it might not actually be created (if it's not existing yet)
+                                Some(local_branch) => {
+                                    // This is always safe because…
+                                    // - the reference may exist already
+                                    // - if it doesn't exist it will be created - we are here because it's in the list of mappings after all
+                                    // - if it exists and is updated, and the update is rejected due to non-fastforward for instance, the
+                                    //   target reference still exists and we can point to it.
+                                    Target::Symbolic(local_branch)
+                                }
                                 None => Target::Peeled(remote_id.into()),
                             }
                         } else {

git-repository/src/remote/connection/fetch/update_refs/tests.rs

@@ -216,20 +216,44 @@ mod update {
     }
 
     #[test]
-    #[ignore]
-    fn remote_symbolic_refs_can_fail_to_be_set_if_their_local_tracking_branch_is_not_existing_or_scheduled_to_exist() {
+    fn remote_symbolic_refs_can_always_be_set_as_there_is_no_scenario_where_it_could_be_nonexisting_and_rejected() {
         let repo = repo("two-origins");
-        let (mappings, specs) = mapping_from_spec("refs/heads/symbolic:refs/remotes/origin/new", &repo);
+        let (mut mappings, specs) = mapping_from_spec("refs/heads/symbolic:refs/remotes/origin/new", &repo);
+        mappings.push(Mapping {
+            remote: Source::Ref(git_protocol::fetch::Ref::Direct {
+                full_ref_name: "refs/heads/main".try_into().unwrap(),
+                object: hex_to_id("f99771fe6a1b535783af3163eba95a927aae21d5"),
+            }),
+            local: Some("refs/heads/symbolic".into()),
+            spec_index: 0,
+        });
         let out = fetch::refs::update(&repo, prefixed("action"), &mappings, &specs, fetch::DryRun::Yes).unwrap();
 
-        assert_eq!(out.edits.len(), 0);
+        assert_eq!(out.edits.len(), 1);
         assert_eq!(
             out.updates,
-            vec![fetch::refs::Update {
-                mode: fetch::refs::update::Mode::RejectedSymbolic,
-                edit_index: None
-            }],
+            vec![
+                fetch::refs::Update {
+                    mode: fetch::refs::update::Mode::New,
+                    edit_index: Some(0)
+                },
+                fetch::refs::Update {
+                    mode: fetch::refs::update::Mode::RejectedSymbolic,
+                    edit_index: None
+                }
+            ],
         );
+        let edit = &out.edits[0];
+        match &edit.change {
+            Change::Update { log, new, .. } => {
+                assert_eq!(log.message, "action: storing ref");
+                assert!(
+                    new.try_name().is_some(),
+                    "remote falls back to peeled id as it's the only thing we seem to have locally, it won't refer to a non-existing local ref"
+                );
+            }
+            _ => unreachable!("only updates"),
+        }
     }
 
     #[test]