Merge ~skia/git-ubuntu:skia/fix_submit into git-ubuntu:main

[Skia]

These three commits are helping out `git ubuntu submit` move out of "EXPERIMENTAL".
It makes its default behavior to just work when submitting a simple change to the `devel` release, without any flag required.
I don't know yet about submitting changes against stable series, that remains to be tested.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:dc7d6336cbfa192a615187b5f2fe345eaf3c40d5
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/85/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Clone
    FAILED: Build

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/85//rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:2877d13537a4f9cf681034e1b62a964f67f994c3
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/86/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Clone
    SUCCESS: Build
    SUCCESS: VM Reset
    FAILED: Unit Tests

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/86//rebuild

review: Needs Fixing (continuous-integration)

[Skia (skia)]

This time the CI failure seems to be on the infra:
```
+ uvt-kvm ssh --insecure gitubuntu-ci-86 -- bash -l -c git-ubuntu.self-test
/snap/git-ubuntu/x1/bin/bash: /snap/git-ubuntu/x1/usr/share/git-ubuntu/self-test: No such file or directory
```

[Robie Basak (racb)]

Please fix CI before landing this!

Thank you for working on this! Comments inline.

review: Needs Fixing

c977f4b... by Skia on 2025-11-18

Add a test for 'nearest_remote_branches'

This test currently fails, the fix is in the next commit.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:85fd7a9fbcba79643df4b855d0e6b1ae3549a7b4
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/87/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Clone
    SUCCESS: Build
    SUCCESS: VM Reset
    FAILED: Unit Tests

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/87//rebuild

review: Needs Fixing (continuous-integration)

03b8bb7... by Skia on 2025-11-20

snap: add more metadata

Silences the following linter outputs:
metadata: Metadata field '<field>' is missing or empty.

7d6c0a8... by Skia on 2025-11-20

snap: explicitely call the 'pack' subcommand

Silences the following warning:
Running snapcraft without a command will not be possible in future releases. Use 'snapcraft pack' instead.

7201004... by Skia on 2025-11-20

Revert "snap: Drop git-ubuntu as a stage-package"

This reverts commit 6684cf84050058a9f77e404240e8361947f5f42d.

This is a WIP to test if CI works or not. Clearly, not having the
`git-ubuntu` package installed breaks the snap, and thus the tests,
because `git-ubuntu.self-test` can't work.
Let's investigate deeper why the build failed at the time of this
commit.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:3c8c569ec46bfdd5197b03a56482141083a88d8b
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/88/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Clone
    SUCCESS: Build
    SUCCESS: VM Reset
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/88//rebuild

review: Approve (continuous-integration)

908a34d... by Skia on 2025-11-24

git_repository: rework '_derive_target_branch_string' API

Since `pygit2.Branch` is not hash-able (anymore?), the use of `set()`
was removed a couple of commits ago, with a very limited set of changes
to avoid changing the API too much.
This commit now introduces a bigger rework, to actually get rid of the
"deduplicated list" hack by using a `dict`.
Thanks @rbasak for the data structure suggestion!

5b1dd31... by Skia on 2025-11-24

git_repository_test: improve coverage of '_derive_target_branch_string'

* Add the few edge cases that weren't covered
* Add the checks on the logging, better showing all the edge cases

[Skia (skia)]

@rbasak: With the last two commits, I believe all your comments are addressed. This MP now comes with multiple additional improvements in both the snap packaging and the tests. See individual commit logs for details.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:2f18e083d0da9cf03655b5c622fb63e7969acde7
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/89/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Clone
    SUCCESS: Build
    SUCCESS: VM Reset
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/89//rebuild

review: Approve (continuous-integration)

[Robie Basak (racb)]

This is looking good! I don't have time to give this a full review, but I noticed some bits that I hope will help make progress. Comments inline.

I also can't see why your snap packaging related commits aren't showing up in https://git.launchpad.net/~skia/git-ubuntu/log/gitubuntu?h=skia/fix_submit, so that's another reason this review isn't complete.

review: Needs Fixing

[Robie Basak (racb)]

> I also can't see why your snap packaging related commits aren't showing up in https://git.launchpad.net/~skia/git-ubuntu/log/gitubuntu?h=skia/fix_submit

Ah, because the /gitubuntu at the end was masking it.

e262e35... by Skia on 2025-12-02

git_repository: rework '_derive_target_branch_string' API to raise upon errors

Previous API was returning an empty string and logging out directly.
That's not a good pattern for a library module.
New API raises a specific exception instead, and let the caller do the
logging itself. This makes the return value of the function to always be
a valid one when the function returns normally.

[Skia (skia)]

I've answered inline, and fixed what could be fixed.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:e262e356fbe5cef0cc4ebdd454476cd3d1572deb
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/92/
Executed test runs:
    SUCCESS: VM Setup
    SUCCESS: Clone
    SUCCESS: Build
    SUCCESS: VM Reset
    SUCCESS: Unit Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.canonical.com/server-team/job/git-ubuntu-ci/92//rebuild

review: Approve (continuous-integration)

Unmerged commits

e262e35... by Skia on 2025-12-02

git_repository: rework '_derive_target_branch_string' API to raise upon errors

Previous API was returning an empty string and logging out directly.
That's not a good pattern for a library module.
New API raises a specific exception instead, and let the caller do the
logging itself. This makes the return value of the function to always be
a valid one when the function returns normally.

5b1dd31... by Skia on 2025-11-24

git_repository_test: improve coverage of '_derive_target_branch_string'

* Add the few edge cases that weren't covered
* Add the checks on the logging, better showing all the edge cases

908a34d... by Skia on 2025-11-24

git_repository: rework '_derive_target_branch_string' API

Since `pygit2.Branch` is not hash-able (anymore?), the use of `set()`
was removed a couple of commits ago, with a very limited set of changes
to avoid changing the API too much.
This commit now introduces a bigger rework, to actually get rid of the
"deduplicated list" hack by using a `dict`.
Thanks @rbasak for the data structure suggestion!

7201004... by Skia on 2025-11-20

Revert "snap: Drop git-ubuntu as a stage-package"

This reverts commit 6684cf84050058a9f77e404240e8361947f5f42d.

This is a WIP to test if CI works or not. Clearly, not having the
`git-ubuntu` package installed breaks the snap, and thus the tests,
because `git-ubuntu.self-test` can't work.
Let's investigate deeper why the build failed at the time of this
commit.

7d6c0a8... by Skia on 2025-11-20

snap: explicitely call the 'pack' subcommand

Silences the following warning:
Running snapcraft without a command will not be possible in future releases. Use 'snapcraft pack' instead.

03b8bb7... by Skia on 2025-11-20

snap: add more metadata

Silences the following linter outputs:
metadata: Metadata field '<field>' is missing or empty.

5838624... by Skia on 2025-11-17

submit: fix the auto-discovery of the devel branch

LP: #1756088
When submitting a simple patch against `ubuntu/devel`, the
auto-discovery mechanism was broken because of the prepended `pkg/`.
Stripping it as for other cases in the function makes the `git ubuntu
submit` command to just work™ in that case.

d5a52f1... by Skia on 2025-11-17

submit: fix traceback when finding target branch

LP: #2087340
On a dead simple call of `git ubuntu submit`:
```
❯ git ubuntu submit
['ubuntu-sponsors']
Traceback (most recent call last):
  File "/snap/git-ubuntu/2124/usr/bin/git-ubuntu", line 33, in <module>
    sys.exit(load_entry_point('gitubuntu==1.2', 'console_scripts', 'git-ubuntu')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/snap/git-ubuntu/2124/usr/lib/python3/dist-packages/gitubuntu/__main__.py", line 270, in main
    sys.exit(args.func(args))
             ^^^^^^^^^^^^^^^
  File "/snap/git-ubuntu/2124/usr/lib/python3/dist-packages/gitubuntu/submit.py", line 77, in cli_main
    return main(
           ^^^^^
  File "/snap/git-ubuntu/2124/usr/lib/python3/dist-packages/gitubuntu/submit.py", line 160, in main
    target_head_string = derive_target_branch(
                         ^^^^^^^^^^^^^^^^^^^^^
  File "/snap/git-ubuntu/2124/usr/lib/python3/dist-packages/gitubuntu/git_repository.py", line 232, in derive_target_branch
    repo.nearest_remote_branches(commitish_string, namespace)
  File "/snap/git-ubuntu/2124/usr/lib/python3/dist-packages/gitubuntu/git_repository.py", line 2154, in nearest_remote_branches
    remote_heads_by_commit[b.peel().id].add(b)
TypeError: unhashable type: '_pygit2.Branch'
```

This commit trades the use of a `set` for the use of a `list` with
manual deduplication (because `pygit2.Branch` isn't hashable, thus can't
be added to a set). That way, only the containing data type is changed,
but the contract on what data to expect is kept.
This probably isn't the most elegant fix, but it's a fix that doesn't
require digging deep into the git-ubuntu API, so I'm fairly confident
this is only improving things, and not breaking too many other use-cases
that I don't know about.

c977f4b... by Skia on 2025-11-18

Add a test for 'nearest_remote_branches'

This test currently fails, the fix is in the next commit.

diff --git a/gitubuntu/git_repository.py b/gitubuntu/git_repository.py
index a447293..52702b3 100644
--- a/gitubuntu/git_repository.py
+++ b/gitubuntu/git_repository.py
@@ -164,34 +164,29 @@ def _derive_git_cli_env(
     return env
 
 
-def _derive_target_branch_string(remote_branch_objects):
-    '''Given a list of branch objects, return the name of the one to use as the target branch
+class TargetBranchError(Exception):
+    pass
+
+
+def _derive_target_branch_string(remote_branch_dict: dict[str, pygit2.Branch]):
+    '''Given a dict of branch_name: branch objects, return the name of the one
+    to use as the target branch
 
     Returns either one of the branch objects' names, or the empty string
     to indicate no suitable candidate.
     '''
-    if len(remote_branch_objects) == 0:
-        logging.error("Unable to automatically determine importer "
-            "branch: No candidate branches found."
-        )
-        return ''
-    remote_branch_strings = [
-        b.branch_name for b in remote_branch_objects
-    ]
-    if len(remote_branch_objects) > 1:
+    if len(remote_branch_dict) == 0:
+        raise TargetBranchError("No candidate branch found.")
+    remote_branch_strings = list(remote_branch_dict.keys())
+    if len(remote_branch_dict) > 1:
         # do the trees of each branch's tip match?
         if len(
-            set(b.peel(pygit2.Tree).id for b in remote_branch_objects)
+            set(b.peel(pygit2.Tree).id for b in remote_branch_dict.values())
         ) != 1:
-            logging.error("Unable to automatically determine importer "
-                "branch: Multiple candidate branches found and "
+            raise TargetBranchError("Multiple candidate branches found and "
                 "their trees do not match: %s. This might be a "
                 "bug in `git ubuntu lint`, please report it at "
-                "https://bugs.launchpad.net/git-ubuntu. "
-                "Please pass  --target-branch.",
-                ", ".join(remote_branch_strings)
-            )
-            return ''
+                "https://bugs.launchpad.net/git-ubuntu. " % ", ".join(remote_branch_strings))
         # is ubuntu/devel one of the candidates?
         try:
             return [
@@ -207,20 +202,12 @@ def _derive_target_branch_string(remote_branch_objects):
             b in remote_branch_strings
         )
         if len(pkg_remote_branch_serieses) != 1:
-            logging.error("Unable to automatically determine importer "
-                "branch: Multiple candidate branches found and "
-                "they do not target the same series: %s. Please pass "
-                "--target-branch.", ", ".join(remote_branch_strings)
-            )
-            return ''
+            raise TargetBranchError("Multiple candidate branches found and "
+                "they do not target the same series: %s." % ", ".join(remote_branch_strings))
         # is a -devel branch present?
         if not any('-devel' in b for b in remote_branch_strings):
-            logging.error("Unable to automatically determine importer "
-                "branch: Multiple candidate branches found and "
-                "none appear to be a -devel branch: %s. Please "
-                "pass  --target-branch.", ", ".join(remote_branch_strings)
-            )
-            return ''
+            raise TargetBranchError("Multiple candidate branches found and "
+                "none appear to be a -devel branch: %s." % ", ".join(remote_branch_strings))
         # if so, favor -devel
         remote_branch_strings = [
             b for b in remote_branch_strings if '-devel' in b
@@ -2130,14 +2117,14 @@ class GitUbuntuRepository:
 
     def nearest_remote_branches(self, commit_hash, prefix=None,
         max_commits=100
-    ):
-        '''Return the set of remote branches nearest to @commit_hash
+    ) -> dict[str, pygit2.Branch]:
+        '''Return a dict of remote branches nearest to @commit_hash
 
-        This is a set of remote branch objects that are currently
+        This is a dict of remote branch objects that are currently
         pointing at a commit, where that commit is the nearest ancestor
         to @commit_hash among the possible commits.
 
-        If no such commit is found, an empty set is returned.
+        If no such commit is found, an empty dict is returned.
 
         Only consider remote branches that start with @prefix.
 
@@ -2148,10 +2135,10 @@ class GitUbuntuRepository:
         '''
 
         # 1) cache all prefixed branch names by commit
-        remote_heads_by_commit = collections.defaultdict(set)
+        remote_heads_by_commit = collections.defaultdict(dict)
         for b in self.remote_branches:
             if prefix is None or b.branch_name.startswith(prefix):
-                remote_heads_by_commit[b.peel().id].add(b)
+                remote_heads_by_commit[b.peel().id][b.branch_name] = b
 
         # 2) walk from commit_hash backwards until a cached commit is found
         commits = self.raw_repo.walk(
@@ -2166,10 +2153,10 @@ class GitUbuntuRepository:
                 return remote_heads_by_commit[commit.id]
 
             # in the currently impossible (but permitted in this state) case
-            # that the dictionary returned an empty set, we loop around again
+            # that the dictionary returned an empty dict, we loop around again
             # which is what we want.
 
-        return set()
+        return dict()
 
 
     def nearest_tag(
diff --git a/gitubuntu/git_repository_test.py b/gitubuntu/git_repository_test.py
index 00c62ba..66f6a20 100644
--- a/gitubuntu/git_repository_test.py
+++ b/gitubuntu/git_repository_test.py
@@ -31,36 +31,41 @@ from gitubuntu.test_fixtures import (
 from gitubuntu.test_util import get_test_changelog
 
 
[e-mail]('same_remote_branch_names, different_remote_branch_names, expected', [
-    ([], [], ''),
-    (['pkg/ubuntu/xenial-devel',], [], 'pkg/ubuntu/xenial-devel'),
-    (['pkg/ubuntu/xenial-security',], [], 'pkg/ubuntu/xenial-security'),
-    (['pkg/ubuntu/xenial-updates', 'pkg/ubuntu/xenial-devel'], [],
-        'pkg/ubuntu/xenial-devel'
-    ),
-    ([], ['pkg/ubuntu/xenial-updates', 'pkg/ubuntu/xenial-devel'],
-        ''
-    ),
-    (['pkg/ubuntu/zesty-devel', 'pkg/ubuntu/zesty-proposed', 'pkg/ubuntu/devel'], [], 'pkg/ubuntu/devel'),
[e-mail]('same_remote_branch_names, different_remote_branch_names, expected, expected_log', [
+    ([], [], None, 'No candidate branches found'),
+    (['pkg/ubuntu/xenial-devel',], [], 'pkg/ubuntu/xenial-devel', ''),
+    (['pkg/ubuntu/xenial-security',], [], 'pkg/ubuntu/xenial-security', ''),
+    (['pkg/ubuntu/xenial-updates', 'pkg/ubuntu/xenial-devel'], [], 'pkg/ubuntu/xenial-devel' , ''),
+    ([], ['pkg/ubuntu/xenial-updates', 'pkg/ubuntu/xenial-devel'], None, 'Multiple candidate branches found and their trees do not match'),
+    (['pkg/ubuntu/zesty-devel', 'pkg/ubuntu/zesty-proposed', 'pkg/ubuntu/devel'], [], 'pkg/ubuntu/devel', ''),
+    (['pkg/ubuntu/zesty-updates', 'pkg/ubuntu/zesty-proposed'], [], None, 'Multiple candidate branches found and none appear to be a -devel branch'),
+    (['pkg/ubuntu/xenial-devel', 'pkg/ubuntu/focal-devel'], [], None, 'Multiple candidate branches found and they do not target the same series'),
 ])
-def test__derive_target_branch_string(same_remote_branch_names,
-    different_remote_branch_names, expected
+def test__derive_target_branch_string(caplog, same_remote_branch_names,
+    different_remote_branch_names, expected, expected_log
 ):
-    remote_branch_objects = []
+    remote_branch_objects = {}
     for branch_name in same_remote_branch_names:
         b = unittest.mock.Mock()
         b.peel(pygit2.Tree).id = unittest.mock.sentinel.same_id
         b.branch_name = branch_name
-        remote_branch_objects.append(b)
+        remote_branch_objects[branch_name] = b
     for branch_name in different_remote_branch_names:
         b = unittest.mock.Mock()
         b.peel(pygit2.Tree).id = object() # need a different sentinel for each
         b.branch_name = branch_name
-        remote_branch_objects.append(b)
-    target_branch_string = target._derive_target_branch_string(
-        remote_branch_objects
-    )
-    assert target_branch_string == expected
+        remote_branch_objects[branch_name] = b
+    if expected:
+        target_branch_string = target._derive_target_branch_string(
+            remote_branch_objects
+        )
+        assert target_branch_string == expected
+    else:
+        with pytest.raises(target.TargetBranchError) as e:
+            target._derive_target_branch_string(
+                remote_branch_objects
+            )
+            assert expected_log in str(e)
 
 
 @pytest.mark.parametrize('changelog_name, expected', [
@@ -1265,3 +1270,43 @@ def test_create_tracking_branch_tracks(repo, pygit2_repo):
     )
     assert repo.raw_repo.config['branch.main.remote'] == 'origin'
     assert repo.raw_repo.config['branch.main.merge'] == 'refs/heads/main'
+
+
+def test_nearest_remote_branches(repo, pygit2_repo):
+    # Note that we cannot use a repo instance twice, because pytest will give
+    # us only one instance. A convenient workaround is that the remote repo
+    # doesn't need to be a GitUbuntuRepository and so the pygit2_repo fixture
+    # will do as it is independent. We can assert they are definitely
+    # different:
+    assert repo.raw_repo is not pygit2_repo
+    base_repo = Repo(
+        commits=[
+            Commit(name='first'),
+            Commit(name='second', parents=[Placeholder('first')]),
+        ],
+        branches={
+            'ubuntu/devel': Placeholder('first'),
+            'ubuntu/resolute': Placeholder('first'),
+            'ubuntu/devel-proposed': Placeholder('second'),
+            'ubuntu/resolute-proposed': Placeholder('second'),
+        },
+    )
+    base_repo.write(pygit2_repo)
+    local_repo = base_repo.copy(
+        add_commits=[
+            Commit.from_spec(
+                parents=[Placeholder('second')],
+                name='third',
+            ),
+        ],
+        update_branches={'new-feature': Placeholder('third')}
+    )
+    local_repo.write(repo.raw_repo)
+
+    repo.raw_repo.remotes.create('origin', pygit2_repo.path)
+    repo.git_run(['fetch', 'origin'])
+
+    commit = repo.raw_repo.references['refs/heads/new-feature'].peel(pygit2.Commit)
+    branch = repo.nearest_remote_branches(str(commit.id))
+    assert len(branch.keys()) == 2
+    assert list(branch.keys()) == ["origin/ubuntu/devel-proposed", "origin/ubuntu/resolute-proposed"]
diff --git a/gitubuntu/submit.py b/gitubuntu/submit.py
index 41ef4b5..0a80465 100644
--- a/gitubuntu/submit.py
+++ b/gitubuntu/submit.py
@@ -12,6 +12,7 @@ from gitubuntu.git_repository import (
     derive_target_branch,
     GitUbuntuRepository,
     git_dep14_tag,
+    TargetBranchError,
 )
 from gitubuntu.run import decode_binary, run
 from gitubuntu.source_information import GitUbuntuSourceInformation, launchpad_login_auth
@@ -157,12 +158,15 @@ def main(
             namespace = 'pkg'
         else:
             namespace = target_user
-        target_head_string = derive_target_branch(
-            repo,
-            commitish_string,
-            namespace,
-        )
-        if len(target_head_string) == 0:
+        try:
+            target_head_string = derive_target_branch(
+                repo,
+                commitish_string,
+                namespace,
+            )
+        except TargetBranchError as e:
+            logging.error("Unable to automatically determine importer branch: %s\n"
+                          "Please pass --target-branch.", str(e))
             return 1
 
     logging.debug("target branch: %s", target_head_string)
@@ -202,6 +206,7 @@ def main(
         )
         return 1
 
+    target_head_string = target_head_string.removeprefix("pkg/")
     target_git_ref = target_git_repo.getRefByPath(
         path='refs/heads/%s' % target_head_string
     )
diff --git a/snap.sh b/snap.sh
index 7817f56..8159cbe 100644
--- a/snap.sh
+++ b/snap.sh
@@ -104,4 +104,4 @@ EOT
 # Build the snap itself
 # Using sudo as a workaround for
 # https://github.com/canonical/snapcraft/issues/5113
-sudo snapcraft --destructive-mode
+sudo snapcraft pack --destructive-mode --verbosity verbose
diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml
index ee08a80..96854db 100644
--- a/snap/snapcraft.yaml
+++ b/snap/snapcraft.yaml
@@ -2,6 +2,12 @@ name: git-ubuntu
 version: git
 summary: Ubuntu development git tooling
 description: Maintain an Ubuntu source package in a git tree
+title: git-ubuntu
+source-code: [ https://code.launchpad.net/~git-ubuntu-dev/git-ubuntu/+git/git-ubuntu ]
+contact: [ https://launchpad.net/~git-ubuntu-dev ]
+license:  GPL-3.0
+issues: [ https://bugs.launchpad.net/git-ubuntu ]
+website: [ https://launchpad.net/git-ubuntu ]
 confinement: classic
 grade: stable
 base: core24
@@ -64,6 +70,7 @@ parts:
             - findutils
             - git
             - git-buildpackage
+            - git-ubuntu
             - gpgv
             - grep
             - gzip