Bug#927348: unblock: salt/2018.3.4+dfsg1-2

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Benjamin Drung-6
Package: release.debian.org
Severity: normal
User: [hidden email]
Usertags: unblock

Please unblock package salt

This version fixes the test_xen_virtual test case (bug #922352) and
exposes tornado4 as tornado for zmq.eventloop.ioloop (bug #924763). Our
salt 2018.3.3+dfsg1-1 package introduced a big patch to use
python3-tornado4 (instead of python3-tornado) due to missing support for
tornado version 5. Without the fix for #924763, zmq.eventloop.ioloop
will import tornado version 5 (if python3-tornado is installed).

I also included fix-various-spelling-mistakes.patch which fixes several
spelling mistakes. Because this patch file is long, I excluded it from the
attached debdiff.

This version also switches from the a pre-release git snapshot to the
official 2018.3.4 release. The only difference between this snapshot and
the release are two commits ("Fix ssh on Windows" and "Update url to
libsodium for mac builds") and that the release tarball ships less files
than what can be found in git.

For that reason, the attached debdiff is created with this command:

debdiff --exclude fix-various-spelling-mistakes.patch
salt_2018.3.4~git20180207+dfsg1-1.dsc salt_2018.3.4+dfsg1-2.dsc |
filterdiff -i '*/debian/*' -i '*/tests/*/test_ssh.py' -i
'*/salt/modules/ssh.py' -i '*/pkg/osx/build_env.sh' >
salt_2018.3.4+dfsg1-2.debdiff

Alternatively this more simple git diff command could be used:

git diff --diff-filter=ACM
debian/2018.3.4_git20180207+dfsg1-1..debian/2018.3.4+dfsg1-2

You can also look at all the individual commits on salsa:
https://salsa.debian.org/salt-team/salt/compare/debian%2F2018.3.4_git20180207+dfsg1-1...debian%2F2018.3.4+dfsg1-2

All 7575 unittest succeeded and I successfully tested this new salt
version on Debian unstable with our production environment setup
(running the highstate on a salt minion connected to the salt master).

unblock salt/2018.3.4+dfsg1-2

-- System Information:
Debian Release: buster/sid
  APT prefers disco
  APT policy: (500, 'disco')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 5.0.0-13-generic (SMP w/12 CPU cores)
Kernel taint flags: TAINT_WARN
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8), LANGUAGE=de_DE:en (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

salt_2018.3.4+dfsg1-2.debdiff (31K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Benjamin Drung-6
retitle 927348 unblock: salt/2018.3.4+dfsg1-4
thanks

Hi,

two more uploads for salt were needed: The first for repairing the
documentation (correct JavaScript symlinks and making the search work
again). The second for fixing the autopkgtest when using systemd 241-3.
A debdiff between salt 2018.3.4+dfsg1-2 and 2018.3.4+dfsg1-4 is
attached.

unblock salt/2018.3.4+dfsg1-4

--
Benjamin Drung
System Developer
Debian & Ubuntu Developer

1&1 IONOS Cloud GmbH | Greifswalder Str. 207 | 10405 Berlin | Germany
E-mail: [hidden email] | Web: www.ionos.de

Head Office: Berlin, Germany
District Court Berlin Charlottenburg, Registration number: HRB 125506 B
Executive Management: Christoph Steffens, Matthias Steinberg, Achim
Weiss

Member of United Internet

salt_2018.3.4+dfsg1-4.debdiff (20K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Paul Gevers-4
In reply to this post by Benjamin Drung-6
Control: tags -1 moreinfo

Hi Benjamin,

On Thu, 18 Apr 2019 13:01:31 +0200 Benjamin Drung
<[hidden email]> wrote:
> This version fixes the test_xen_virtual test case (bug #922352) and
> exposes tornado4 as tornado for zmq.eventloop.ioloop (bug #924763). Our
> salt 2018.3.3+dfsg1-1 package introduced a big patch to use
> python3-tornado4 (instead of python3-tornado) due to missing support for
> tornado version 5. Without the fix for #924763, zmq.eventloop.ioloop
> will import tornado version 5 (if python3-tornado is installed).

Both bugs have severity normal. Do you really want to bother now or is
the severity not correct (then please fix that and elaborate)?

> I also included fix-various-spelling-mistakes.patch which fixes several
> spelling mistakes. Because this patch file is long, I excluded it from the
> attached debdiff.

Bugs can be introduced that way. I am not going to review that diff,
fixing spelling mistakes at this moment isn't appropriate unless these
mistakes are crucial somewhere.

> This version also switches from the a pre-release git snapshot to the
> official 2018.3.4 release. The only difference between this snapshot and
> the release are two commits ("Fix ssh on Windows" and "Update url to
> libsodium for mac builds") and that the release tarball ships less files
> than what can be found in git.

If that was all (salt/modules/ssh.py and
tests/integration/modules/test_ssh.py), I could except it. But with less
files, there is also a changes that ...

> For that reason, the attached debdiff is created with this command:
>
> debdiff --exclude fix-various-spelling-mistakes.patch
> salt_2018.3.4~git20180207+dfsg1-1.dsc salt_2018.3.4+dfsg1-2.dsc |
> filterdiff -i '*/debian/*' -i '*/tests/*/test_ssh.py' -i
> '*/salt/modules/ssh.py' -i '*/pkg/osx/build_env.sh' >
> salt_2018.3.4+dfsg1-2.debdiff
>
> Alternatively this more simple git diff command could be used:
>
> git diff --diff-filter=ACM
> debian/2018.3.4_git20180207+dfsg1-1..debian/2018.3.4+dfsg1-2
>
> You can also look at all the individual commits on salsa:
> https://salsa.debian.org/salt-team/salt/compare/debian%2F2018.3.4_git20180207+dfsg1-1...debian%2F2018.3.4+dfsg1-2
>
> All 7575 unittest succeeded and I successfully tested this new salt
> version on Debian unstable with our production environment setup
> (running the highstate on a salt minion connected to the salt master).
>
> unblock salt/2018.3.4+dfsg1-2
You didn't even elaborate on all the (at this phase of the release
inappropriate) changes to the packaging. There is even a newer version
than the one you already mention in a follow up in this bug.

I am not going to unblock this package, and seen the amount of time your
request stayed open and the proposed changes, I don't think my
colleagues are tempted either. I see that salt is marked for
autoremoval. I suggest you aim for a targeted fix.

Paul


signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Benjamin Drung-6
Control: tags -1 - moreinfo

Hi,

Am Donnerstag, den 02.05.2019, 21:05 +0200 schrieb Paul Gevers:

> Control: tags -1 moreinfo
>
> Hi Benjamin,
>
> On Thu, 18 Apr 2019 13:01:31 +0200 Benjamin Drung
> <[hidden email]> wrote:
> > This version fixes the test_xen_virtual test case (bug #922352) and
> > exposes tornado4 as tornado for zmq.eventloop.ioloop (bug #924763).
> > Our
> > salt 2018.3.3+dfsg1-1 package introduced a big patch to use
> > python3-tornado4 (instead of python3-tornado) due to missing
> > support for
> > tornado version 5. Without the fix for #924763,
> > zmq.eventloop.ioloop
> > will import tornado version 5 (if python3-tornado is installed).
>
> Both bugs have severity normal. Do you really want to bother now or
> is the severity not correct (then please fix that and elaborate)?

Bug #922352 can cause a build failure (and does on Ubuntu). Therefore I
raised it to serious.

Determining the severity of bug #924763 is more complicated. The
reporter stumbled over a warning spit out by salt. The warning message
by itself is more or less harmless, but the underlying problem of the
wrong import might have bad effects. I haven't seen any yet, but they
might be there. IMO we shouldn't release salt with an issue introduced
by one of our patches.

> > I also included fix-various-spelling-mistakes.patch which fixes
> > several
> > spelling mistakes. Because this patch file is long, I excluded it
> > from the
> > attached debdiff.
>
> Bugs can be introduced that way. I am not going to review that diff,
> fixing spelling mistakes at this moment isn't appropriate unless
> these mistakes are crucial somewhere.

I can drop that patch again when this is the only blocker for getting
the unblock request accepted.

> > This version also switches from the a pre-release git snapshot to
> > the
> > official 2018.3.4 release. The only difference between this
> > snapshot and
> > the release are two commits ("Fix ssh on Windows" and "Update url
> > to
> > libsodium for mac builds") and that the release tarball ships less
> > files
> > than what can be found in git.
>
> If that was all (salt/modules/ssh.py and
> tests/integration/modules/test_ssh.py), I could except it. But with
> less files, there is also a changes that ...

All previous upstream releases like 2017.7.3, 2017.7.2, and so on did
not contain these additional files that the git snapshot
2018.3.4~git20180207 contained. All these additional, auxiliary files
are not needed for the installation. I should have used the upstream
method to create the release tarball instead of using "git archive"
when creating the 2018.3.4~git20180207 tarball.

> > For that reason, the attached debdiff is created with this command:
> >
> > debdiff --exclude fix-various-spelling-mistakes.patch
> > salt_2018.3.4~git20180207+dfsg1-1.dsc salt_2018.3.4+dfsg1-2.dsc |
> > filterdiff -i '*/debian/*' -i '*/tests/*/test_ssh.py' -i
> > '*/salt/modules/ssh.py' -i '*/pkg/osx/build_env.sh' >
> > salt_2018.3.4+dfsg1-2.debdiff
> >
> > Alternatively this more simple git diff command could be used:
> >
> > git diff --diff-filter=ACM
> > debian/2018.3.4_git20180207+dfsg1-1..debian/2018.3.4+dfsg1-2
> >
> > You can also look at all the individual commits on salsa:
> > https://salsa.debian.org/salt-team/salt/compare/debian%2F2018.3.4_git20180207+dfsg1-1...debian%2F2018.3.4+dfsg1-2
> >
> > All 7575 unittest succeeded and I successfully tested this new salt
> > version on Debian unstable with our production environment setup
> > (running the highstate on a salt minion connected to the salt
> > master).
> >
> > unblock salt/2018.3.4+dfsg1-2
>
> You didn't even elaborate on all the (at this phase of the release
> inappropriate) changes to the packaging. There is even a newer
> version
> than the one you already mention in a follow up in this bug.

Which changes to the packaging do you refer to?

Running the tests with LC_ALL=C.UTF-8 fixes a build failure in case the
building machine uses an ANSII locale, which would be worth another RC
bug report.

Upload 2018.3.4+dfsg1-3 repairs the documentation in salt-doc. It
ensures that the pre-built minified Javascript and CSS files are not
leaked into the salt-doc binary package and that all created symlink
are correct. Before this version, salt-doc contained broken symlinks
and the search did not work.

systemd 241 broke salt. Upload 2018.3.4+dfsg1-4 fixes that. There were
no bug report opened for it, but it would be worth one RC bug.

The newly opened RC bug #928337 was fixed in 2018.3.4+dfsg1-5.

--
Benjamin Drung
System Developer
Debian & Ubuntu Developer

1&1 IONOS Cloud GmbH | Greifswalder Str. 207 | 10405 Berlin | Germany
E-mail: [hidden email] | Web: www.ionos.de

Head Office: Berlin, Germany
District Court Berlin Charlottenburg, Registration number: HRB 125506 B
Executive Management: Christoph Steffens, Matthias Steinberg, Achim
Weiss

Member of United Internet

Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Paul Gevers-4
Control: tags -1 moreinfo

Hi Benjamin,

On 06-05-2019 12:47, Benjamin Drung wrote:

> Am Donnerstag, den 02.05.2019, 21:05 +0200 schrieb Paul Gevers:
>> Control: tags -1 moreinfo
>>
>> Hi Benjamin,
>>
>> On Thu, 18 Apr 2019 13:01:31 +0200 Benjamin Drung
>> <[hidden email]> wrote:
>>> This version fixes the test_xen_virtual test case (bug #922352) and
>>> exposes tornado4 as tornado for zmq.eventloop.ioloop (bug #924763).
>>> Our
>>> salt 2018.3.3+dfsg1-1 package introduced a big patch to use
>>> python3-tornado4 (instead of python3-tornado) due to missing
>>> support for
>>> tornado version 5. Without the fix for #924763,
>>> zmq.eventloop.ioloop
>>> will import tornado version 5 (if python3-tornado is installed).
>>
>> Both bugs have severity normal. Do you really want to bother now or
>> is the severity not correct (then please fix that and elaborate)?
>
> Bug #922352 can cause a build failure (and does on Ubuntu). Therefore I
> raised it to serious.
>
> Determining the severity of bug #924763 is more complicated. The
> reporter stumbled over a warning spit out by salt. The warning message
> by itself is more or less harmless, but the underlying problem of the
> wrong import might have bad effects. I haven't seen any yet, but they
> might be there. IMO we shouldn't release salt with an issue introduced
> by one of our patches.
Ack.

>>> I also included fix-various-spelling-mistakes.patch which fixes
>>> several
>>> spelling mistakes. Because this patch file is long, I excluded it
>>> from the
>>> attached debdiff.
>>
>> Bugs can be introduced that way. I am not going to review that diff,
>> fixing spelling mistakes at this moment isn't appropriate unless
>> these mistakes are crucial somewhere.
>
> I can drop that patch again when this is the only blocker for getting
> the unblock request accepted.
Please do.

[...]

>> You didn't even elaborate on all the (at this phase of the release
>> inappropriate) changes to the packaging. There is even a newer
>> version
>> than the one you already mention in a follow up in this bug.
>
> Which changes to the packaging do you refer to?

The once you discuss below.

> Running the tests with LC_ALL=C.UTF-8 fixes a build failure in case the
> building machine uses an ANSII locale, which would be worth another RC
> bug report.

Ack.

> Upload 2018.3.4+dfsg1-3 repairs the documentation in salt-doc. It
> ensures that the pre-built minified Javascript and CSS files are not
> leaked into the salt-doc binary package and that all created symlink
> are correct. Before this version, salt-doc contained broken symlinks
> and the search did not work.

I am very uncomfortable with these changes at this moment.

> systemd 241 broke salt. Upload 2018.3.4+dfsg1-4 fixes that. There were
> no bug report opened for it, but it would be worth one RC bug.

Ack.

> The newly opened RC bug #928337 was fixed in 2018.3.4+dfsg1-5.

Ack.

So, all in all, I don't want to unblock the new upstream with your
packaging, we're too late in the cycle and the version really doesn't
match the freeze policy. However, I am offering you an upload based on
the version currently in buster via t-p-u. If you go that route, please
only fix bugs 919849, 928337, 922352, 924763, the LC_ALL=C.UTF-8 item
and the systemd issue. Please fix 919849 without switching your build to
sphinxdoc, that isn't appropriate at this moment.

You can remove the moreinfo tag when you either have a debdiff ready, or
if your confident, uploaded to buster.

Paul


signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Benjamin Drung-6
Hi Paul,

Am Freitag, den 10.05.2019, 20:40 +0200 schrieb Paul Gevers:

> Control: tags -1 moreinfo
>
> So, all in all, I don't want to unblock the new upstream with your
> packaging, we're too late in the cycle and the version really doesn't
> match the freeze policy. However, I am offering you an upload based
> on the version currently in buster via t-p-u. If you go that route,
> please only fix bugs 919849, 928337, 922352, 924763, the
> LC_ALL=C.UTF-8 item and the systemd issue. Please fix 919849 without
> switching your build to sphinxdoc, that isn't appropriate at this
> moment.

Okay. So you prefer an upload to t-p-u or can I just revert those
rejected changes (typos fixes and using dh_sphinxdoc) and do another
upload to unstable?

--
Benjamin Drung
System Developer
Debian & Ubuntu Developer

1&1 IONOS Cloud GmbH | Greifswalder Str. 207 | 10405 Berlin | Germany
E-mail: [hidden email] | Web: www.ionos.de

Head Office: Berlin, Germany
District Court Berlin Charlottenburg, Registration number: HRB 125506 B
Executive Management: Christoph Steffens, Matthias Steinberg, Achim
Weiss

Member of United Internet

Reply | Threaded
Open this post in threaded view
|

Bug#927348: unblock: salt/2018.3.4+dfsg1-2

Paul Gevers-4
Hi Benjamin,

On 14-05-2019 13:13, Benjamin Drung wrote:

> Hi Paul,
>
> Am Freitag, den 10.05.2019, 20:40 +0200 schrieb Paul Gevers:
>> Control: tags -1 moreinfo
>>
>> So, all in all, I don't want to unblock the new upstream with your
>> packaging, we're too late in the cycle and the version really doesn't
>> match the freeze policy. However, I am offering you an upload based
>> on the version currently in buster via t-p-u. If you go that route,
>> please only fix bugs 919849, 928337, 922352, 924763, the
>> LC_ALL=C.UTF-8 item and the systemd issue. Please fix 919849 without
>> switching your build to sphinxdoc, that isn't appropriate at this
>> moment.
>
> Okay. So you prefer an upload to t-p-u or can I just revert those
> rejected changes (typos fixes and using dh_sphinxdoc) and do another
> upload to unstable?
I would prefer an upload to unstable if you also revert to the previous
upstream tar ball (e.g. using the +really version syntax).

Paul


signature.asc (499 bytes) Download Attachment