launchpad-reviewers team mailing list archive

[Ines Almeida <mp+469997@xxxxxxxxxxxxxxxxxx>]

Just a couple of small comments, this seems like a good idea.

Diff comments:

> diff --git a/debian/changelog b/debian/changelog
> index cc5ceb1..136911c 100644
> --- a/debian/changelog
> +++ b/debian/changelog
> @@ -1,3 +1,10 @@
> +launchpad-buildd (240) UNRELEASED; urgency=medium

Replace UNRELEASED with focal?

> +
> +  * Prefer `--single-branch` over `--no-single-branch` when cloning
> +    repositories to avoid branch scalability issues. Fixes LP:#2067047
> +
> + -- Jürgen Gmach <juergen.gmach@xxxxxxxxxxxxx>  Wed, 24 Jul 2024 08:22:42 +0200
> +
>  launchpad-buildd (239) focal; urgency=medium
>  
>    * Restart snapd after certificate installation and proxy initialization.
> diff --git a/lpbuildd/target/vcs.py b/lpbuildd/target/vcs.py
> index 6df9a1a..b5bc949 100644
> --- a/lpbuildd/target/vcs.py
> +++ b/lpbuildd/target/vcs.py
> @@ -78,17 +86,17 @@ class VCSOperationMixin(StatusOperationMixin):
>              cmd = ["git", "clone", "-n"]
>              if quiet:
>                  cmd.append("-q")
> -            if git_shallow_clone:
> -                cmd.extend(["--depth", "1", "--no-single-branch"])
> +            git_path = self.args.git_path
> +            if self.args.git_path is None:
> +                git_path = "HEAD"
> +            if git_shallow_clone_with_single_branch:
> +                cmd.extend(["--depth", "1", "-b", git_path, "--single-branch"])

Opinion: I agree it's best to have a clear variable name then a short one, but this one is looking pretty long. It feels to me that this is still a shallow clone, just done better, so I'd consider keeping it. But it's not a blocker, just an opinion :)

>              cmd.extend([self.args.git_repository, name])
>              if not self.ssl_verify:
>                  env["GIT_SSL_NO_VERIFY"] = "1"
>          self.backend.run(cmd, cwd=cwd, env=full_env)
>          if self.args.git_repository is not None:
>              repository = os.path.join(cwd, name)
> -            git_path = self.args.git_path

Afaics, if `self.args.branch` is None, then `git_path` doesn't get attributed, and things might fail here. Can you check?

> -            if self.args.git_path is None:
> -                git_path = "HEAD"
>              self.backend.run(
>                  ["git", "checkout", "-q", git_path],
>                  cwd=repository,


-- 
https://code.launchpad.net/~jugmac00/launchpad-buildd/+git/launchpad-buildd/+merge/469997
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad-buildd:fix-branch-scalability-issues into launchpad-buildd:master.