[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Salsa - best thing in Debian in recent years? (Re: finally end single-person maintainership)



Hi,

On 5/20/24 04:32, Otto Kekäläinen wrote:

I agree that duplication is bad - but I disagree that use of version
control duplicates the use of the Debian archive for source code
storage, or that use of GitLab for code reviews would duplicate
Debbugs.

Outside of DM uploads, I'm not sure that there is much of a need for a code review on packaging -- really what we want is to reduce the amount of code for packaging, by moving it into declarative frameworks whenever possible, so the vast majority of packaging changes should be trivial ones like upgrading a dependency version.

Would you be kind and try to understand the opposing viewpoint by
trying it for one day?

I am using it for most of my packages. It has not made my life easier, it's just another layer that I need to communicate my intentions through.

I generally do test builds of my packages in a local pbuilder instance, with a hook script to drop me into a shell if the build fails, so the workspace is kept for my investigation. The only CI system that offers a similar feature is Jenkins, but even there I can only inspect the files through a clunky web interface, as soon as I need to look at a binary file or search for a string, I need to download it as a zipfile, and re-running commands inside the same environment to test them is completely out.

You could go to
https://salsa.debian.org/debbugs-team/debbugs/-/merge_requests/19 and
conduct a code review?

At first glance, looks good to me.

Looking at the changes:

1. The outdated build dependency is not in the package currently in Debian. If it was, it would have been spotted by Debian's archive analysis tools already, without the need for a build attempt.

This static analysis is cheaper than a rebuild, so to achieve the same level of coverage, Salsa would need to perform a full archive rebuild daily, and it would still not catch the broken Suggests: in the binary.

2. The missing file in debian/docs was already reported as #903413.

3. The other changes are "upstream" changes, which should have a separate CI that is more extensive than "still builds."

Native packages should only be used for things where it does not make sense to maintain a separate upstream package because they only exist within the package ecosystem, like the "menu" package. Debbugs should really be split into separate "upstream" and "packaging" efforts.

You might discover that GitLab is useful and is not duplicating
Debbugs or anything else in Debian

Well, there is an issue tracker (where tickets go unresponded for a year), that is certainly a duplication of debbugs. It would make sense to maybe track "upstream" bugs there and forward them from debbugs (a feature not present in GitLab's issues handling, but important for package maintenance).

 - it is currently the only platform
to conduct code reviews on in a way that has automatic testing and
comment-response-resolved -tracking. Doing code reviews patches
attached in email does not have that.

Well, I take the diff, prepend each line with > and insert my comments, then send it back. The author then responds to that email, and once the discussion is over, I get a new proposed patch. Not much difference.

If you try it out, and still think Salsa is bad for Debian, then I am
more willing to accept your stanze.

It's not *bad*, but for a lot of our workflows, it's the wrong tool because the use cases it was designed for are different from ours, and there is little we can do to make them meet.

Debian's workflow for collaboration on things that are not yet release-ready is clunky to the point that almost no one uses it that way, but in principle it is there: one can always upload a package to experimental and get it autobuilt on the major architectures, and other DDs can download it from there if they want to take a look at it.

This workflow is what packaging in git mostly replaces: in pkg-electronics, we quite often have changes that are not ready for release that we want to distribute to the other team members. Quite often, these changes do not build straight away, and the reason they are shared is specifically so other people can take a look at them.

Git is a lot better for fostering this collaboration than uploads to experimental, because we get change tracking for individual files, which is invaluable when dealing with a behemoth like ghdl that takes a few hours to build and run tests.

The review process still takes place via mail here, because part of the process is that everyone involved needs to be able to build the package locally and investigate failures. We can quickly incorporate changes from others using git and do a minimal rebuild locally, that is useful, but this essentially means that we are pushing commits to an offside branch.

Attaching the discussion to individual changes is not that useful for us:

1. changing an annotated line in a commit hides the annotation when looking at another commit, so the entire discussion would need to take place in the "all changes" view, or we risk losing context.

2. a lot of the discussion is "things that will need to be changed", not "things that have been changed and we don't like it." GitLab does not have a workflow for discussing that as part of a MR.

In that process, CI would only tell us that the package fails to build, but we already know that: that's why we shared it in this way. If it worked, we would have uploaded it already. Trying to build it in CI is a waste of four hours of CPU time, and we iterate a lot faster than that usually because we do incremental builds in between, and a full build inside pbuilder only as part of the final upload procedure.

After an upload is done, the discussion and intermediate stages become less useful for us, so GitLab's approach of isolating them in the MR is acceptable from my point of view, even if other people would like to have them archived in the mailing list archive so it is easily accessible to search engines.

Also, we usually get build failures from ports, which is kind of unavoidable because CI testing everywhere is prohibitively expensive, and the high level of optimization we need to run means that we will get bitten by target specific bugs. We cannot avoid uploading "broken" packages, unless we integrate the autobuilder network into CI and wait four days for jobs to finish.

What Debian does instead is to filter broken packages from reaching testing -- this is way stricter than any CI process on Salsa can provide, although with a longer turnaround time, because the CI processes performed by the Debian archive software take a lot longer.

This, too, is something that Salsa cannot replicate because of the way GitLab is designed, so it cannot supplant this process, just duplicate some aspects of it, so we would end up with build failure reports from two sources, in two different places.

There is likely a spot where collaborating on packages through MRs is useful, but I'd argue that the majority of packages will either get only trivial MRs that don't require discussion, or require workflows that cannot be easily mapped to MRs, and attempting to make the workflow fit the tool rather than the other way around will create additional friction here.

   Simon


Reply to: