merge with debian

Bug #1524165 reported by Michael Hudson-Doyle
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
golang (Ubuntu)
Fix Released
Wishlist
Unassigned

Bug Description

I'm attaching a debdiff that merges the latest version from Debian. Well sort of: it also includes a pile of things that should really be in Debian, but haven't been fixed there yet -- see the list of Debian bugs mentioned in the changelog and the patches in them. But it gets the rewritten debian/rules from Debian, which is much simpler and clearer.

I've uploaded this to my ppa at https://launchpad.net/~mwhudson/+archive/ubuntu/ppa/+packages so it at least builds on amd64, i386 and ppc64el and I've stared at the debdiffs for the amd64 debs and they look OK to me.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Also see bug #1524175

Mathew Hodson (mhodson)
Changed in golang (Ubuntu):
importance: Undecided → Wishlist
Revision history for this message
Steve Langasek (vorlon) wrote :

The debdiff shows some changes that are not part of the existing diff between Ubuntu and Debian, e.g. in the 0ubuntu1->4ubuntu1 debdiff:

-Build-Depends: debhelper (>= 7.4.10), bison, ed, mawk | awk, perl, netbase,
- golang (>= 1:1.4~) | gccgo-5
-Build-Depends-Indep: po-debconf
+Build-Depends: debhelper (>= 7.4.10),
+ golang-go (>= 2:1.4.2-2~) | gccgo-5,
+ netbase

These appear to be reasonable changes to make to the packaging, but it's incorrect to list them as "remaining changes" in the debian changelog for the merge. It also complicates the review of the package, since this combines the merge of the new Debian package version, with a number of independent changes.

Could we instead do a clean merge of Debian -4 into xenial, so we can account for everything that's currently in the Ubuntu delta, and then apply any further improvements on top of that? Doing a test merge here gave me several differences to your package, and it would be clearer to understand if these are the result of a disagreement in the merge, or the result of an additional change.

Revision history for this message
Steve Langasek (vorlon) wrote :

- debian/helpers/goenv.sh - this had previously been dropped from the Ubuntu packaging (superseded by the changes to debian/rules), but is readded in your version.
- debian/control: Debian's golang package has dependencies on golang-{doc,go,src} (= ${source:Version}); Debian has golang-{doc,go,src} (= ${binary:Version}). This difference is not explained in the Ubuntu changelog. I believe the delta should be dropped. It happens that both Debian and Ubuntu are wrong, *some* of these deps should be ${source:Version} and *some* of them should be ${binary:Version}. But the difference between the two is irrelevant in Ubuntu, it only matters in Debian - so we should stay in sync with Debian for this bit, and propose a fix in Debian.
- debian/patches/skip-userns-tests-when-chrooted.patch: seems to be newly-added in this revision.
- debian/rules: your merge results look very different than mine, this warrants a closer look since the pre-existing delta was quite large.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote : Re: [Bug 1524165] Re: merge with debian

On 15 December 2015 at 14:29, Steve Langasek
<email address hidden> wrote:
> The debdiff shows some changes that are not part of the existing diff
> between Ubuntu and Debian, e.g. in the 0ubuntu1->4ubuntu1 debdiff:
>
> -Build-Depends: debhelper (>= 7.4.10), bison, ed, mawk | awk, perl, netbase,
> - golang (>= 1:1.4~) | gccgo-5
> -Build-Depends-Indep: po-debconf
> +Build-Depends: debhelper (>= 7.4.10),
> + golang-go (>= 2:1.4.2-2~) | gccgo-5,
> + netbase
>
> These appear to be reasonable changes to make to the packaging, but it's
> incorrect to list them as "remaining changes" in the debian changelog
> for the merge.

Yes, true. They would be remaining changes if the Debian maintainers
merged my patches...

> It also complicates the review of the package, since
> this combines the merge of the new Debian package version, with a number
> of independent changes.

> Could we instead do a clean merge of Debian -4 into xenial, so we can
> account for everything that's currently in the Ubuntu delta, and then
> apply any further improvements on top of that?

Well, maybe. I could certainly drop the build-deps, the copyright
changes, the nocheck support and the lintian warnings for now but the
two patches are required to avoid ftbfs and the race file stuff is a
gross bug I'd rather not import. I'll give it a go.

> Doing a test merge here
> gave me several differences to your package, and it would be clearer to
> understand if these are the result of a disagreement in the merge, or
> the result of an additional change.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On 15 December 2015 at 14:41, Steve Langasek
<email address hidden> wrote:
> - debian/helpers/goenv.sh - this had previously been dropped from the Ubuntu packaging (superseded by the changes to debian/rules), but is readded in your version.

I think m-o-m is confusing you. The file is new in debian since we
uploaded the first 1.5 version to wily.

> - debian/control: Debian's golang package has dependencies on golang-{doc,go,src} (= ${source:Version}); Debian has golang-{doc,go,src} (= ${binary:Version}). This difference is not explained in the Ubuntu changelog. I believe the delta should be dropped. It happens that both Debian and Ubuntu are wrong, *some* of these deps should be ${source:Version} and *some* of them should be ${binary:Version}. But the difference between the two is irrelevant in Ubuntu, it only matters in Debian - so we should stay in sync with Debian for this bit, and propose a fix in Debian.

OK. I think the fact that some all packages become any packages as
part of the golang -> gccgo fake package stuff has impact here?

> - debian/patches/skip-userns-tests-when-chrooted.patch: seems to be newly-added in this revision.

Yeah, but I'm fairly the tests will fail without it (and running the
test suite on build is new in Debian recently). I don't know why they
don't fail on debian buildds -- older kernels there, maybe?

> - debian/rules: your merge results look very different than mine, this warrants a closer look since the pre-existing delta was quite large.

Well what I did was mostly take the Debian rules file and re-implement
the dependency packages afresh on top of the new file. I don't know if
it's entirely honest to call this a "merge" but I definitely think
it's an improvement. What does yours look like?

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

New debdiff (against ubuntu)

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Here are the changes from Debian. Looking reasonably tidy to me.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

>> - debian/control: Debian's golang package has dependencies on golang-{doc,go,src} (= ${source:Version}); Debian has golang-{doc,go,src} (= ${binary:Version}). This difference is not explained in the Ubuntu changelog. I believe the delta should be dropped. It happens that both Debian and Ubuntu are wrong, *some* of these deps should be ${source:Version} and *some* of them should be ${binary:Version}. But the difference between the two is irrelevant in Ubuntu, it only matters in Debian - so we should stay in sync with Debian for this bit, and propose a fix in Debian.
>
> OK. I think the fact that some all packages become any packages as
> part of the golang -> gccgo fake package stuff has impact here?

Oh but if the difference between the two is irrelevant in Ubuntu, who cares :-)

I dropped all these changes.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Oh and one final comment and I'll shut up for today: Debian is in the process of uploading 1.5.2. If they actually do that soon, I'll redo the merge on top of that (which would be nice, because I can drop two patches).

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Debian uploaded 1.5.2 at last, here's the diff from Debian to an updated merge (it's kinda big of course, I'll uploaded the diff from Debian to this as well).

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Diff from Debian's 1.5.2-1 to my package.

Revision history for this message
Steve Langasek (vorlon) wrote :

Hi Michael,

The debdiff for 1.5.2-1ubuntu1 largely looks correct to me. I have a few small corrections:

- the changes to debian/copyright should be retained, these fix a real bug and should be upstreamed to Debian.
- the changes to debian/source/lintian-overrides should be retained, for the same reason.
- the debian/control contents have diverged from Debian quite a bit in matters of field ordering, whitespace, etc. we can lose much of this delta with no change in semantics, so I think it's best to do this now when merging.
- Debian has added versioned Conflicts against golang-golang-x-tools, and golang-go.tools. This is incorrect, these should be versioned Breaks/Replaces to ensure smooth upgrades with apt. Since the Ubuntu package already had Breaks/Replaces, I restored this usage with updated version numbers.
- The golang-src package in Ubuntu Breaks/Replaces ancient versions of golang-go, but this has been dropped in Debian. Unfortunately, these versions are not so ancient that they aren't newer than the version present in the trusty release pocket, so we should hang on to this until xenial is released.
- the changelog should document those changes that have been dropped and why (e.g., patches that are upstream and no longer needed here).
- Debian has added ppc64 to their list of archs where they build golang; so we should extend our list of "go archs" to include this one for better upstreamability to Debian.

Please let me know whether you agree with these changes (in the attached debdiff) or whether you think further revision is needed before upload.

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Hi Steve,

Thanks for the very thorough review!

On 15 January 2016 at 16:47, Steve Langasek
<email address hidden> wrote:
> Hi Michael,
>
> The debdiff for 1.5.2-1ubuntu1 largely looks correct to me. I have a
> few small corrections:
>
> - the changes to debian/copyright should be retained, these fix a real bug and should be upstreamed to Debian.

Already done: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=807304
(so I guess this bug number should be added to d/copyright?)

> - the changes to debian/source/lintian-overrides should be retained, for the same reason.

OK.

> - the debian/control contents have diverged from Debian quite a bit in matters of field ordering, whitespace, etc. we can lose much of this delta with no change in semantics, so I think it's best to do this now when merging.

Thanks.

> - Debian has added versioned Conflicts against golang-golang-x-tools, and golang-go.tools. This is incorrect, these should be versioned Breaks/Replaces to ensure smooth upgrades with apt. Since the Ubuntu package already had Breaks/Replaces, I restored this usage with updated version numbers.

I did wonder about this and assumed Debian was more likely to be
correct than me. Apparently not!

> - The golang-src package in Ubuntu Breaks/Replaces ancient versions of golang-go, but this has been dropped in Debian. Unfortunately, these versions are not so ancient that they aren't newer than the version present in the trusty release pocket, so we should hang on to this until xenial is released.

Er, yes they are? The versions being broke/replaced is 2:1-3~ and
trusty has 2:1.2.1-2ubuntu1 (which has the breaks/replaces of 2:1-3~
already). So I think this really can be dropped (it even seems
*precise* has a newer version than 1-3 :-)).

Of course it's pretty harmless to keep too.

> - the changelog should document those changes that have been dropped and why (e.g., patches that are upstream and no longer needed here).

OK. Thanks.

> - Debian has added ppc64 to their list of archs where they build golang; so we should extend our list of "go archs" to include this one for better upstreamability to Debian.

Good spotting.

> Please let me know whether you agree with these changes (in the attached
> debdiff) or whether you think further revision is needed before upload.

I guess I'd ask you to:

1) re-delete the breaks of golang-src
2) remove mention of same from changelog
3) add the bug number for the copyright changes to changelog

and upload away! And then we can do the 1.5.3 merge, but m-o-m should
get that right.

Cheers,
mwh

> ** Patch added: "golang-1524164-mergier-slangasek.diff"
> https://bugs.launchpad.net/ubuntu/+source/golang/+bug/1524165/+attachment/4550305/+files/golang-1524164-mergier-slangasek.diff
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1524165
>
> Title:
> merge with debian
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/golang/+bug/1524165/+subscriptions

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package golang - 2:1.5.2-1ubuntu1

---------------
golang (2:1.5.2-1ubuntu1) xenial; urgency=low

  * Merge from Debian unstable (LP: #1524165). Remaining changes:
    - d/patches/armhf-elf-header.patch (see upstream bug
      https://github.com/golang/go/issues/7094)
    - debian/copyright: updated copyright file to fix some lintian warnings
      (see Debian bug #807304)
    - Build empty golang-go and golang-src packages on architectures without
      golang support and depend on gccgo instead (see Debian bug #780355)
    - Do not distribute un-built from source race detector runtime files and
      recommend golang-race-detector-runtime instead (see Debian bug #807455)
    - debian/source/lintian-overrides: silence some extra source-missing false
      positives.
    - Breaks/Replaces: older golang-golang-x-tools, not Conflicts, to ensure
      smooth upgrades.
  * Dropped changes, included upstream:
    - d/patches/qemu-compat.patch
    - d/patches/support-new-relocations.patch
  * Add a patch to avoid build failures:
    - d/patches/skip-userns-tests-when-chrooted.patch (see Debian bug #807303)
  * Do not include race enabled packages in golang-go deb (see Debian bug
    #807294)

golang (2:1.5.2-1) unstable; urgency=medium

  * Update to 1.5.2 upstream release (Closes: #807136)

golang (2:1.5.1-4) unstable; urgency=medium

  * Add Conflicts to force newer golang-go.tools too (Closes: #803559)

golang (2:1.5.1-3) unstable; urgency=medium

  * Remove architecture qualification on golang-go Build-Depend now that
    golang-go is available for more architectures.

golang (2:1.5.1-2) unstable; urgency=medium

  * Add Conflicts to force newer golang-golang-x-tools (Closes: #802945).

golang (2:1.5.1-1) unstable; urgency=medium

  * Upload to unstable.
  * Update to 1.5.1 upstream release (see notes from experimental uploads for
    what's changed).
  * Skip tests on architectures where the tests fail.

 -- Michael Hudson-Doyle <email address hidden> Thu, 07 Jan 2016 10:11:50 +1300

Changed in golang (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.