Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.debian.maint.python > #14552 > unrolled thread

review for pipenv/2022.10.12-1

Back to article view | Back to linux.debian.maint.python

Contents

  review for pipenv/2022.10.12-1 Jeroen Ploemen <jcfp@debian.org> - 2022-10-22 19:40 +0200
    Re: review for pipenv/2022.10.12-1 Ileana Dumitrescu <ileanadumitrescu95@gmail.com> - 2022-10-26 16:50 +0200
      Re: review for pipenv/2022.10.12-1 Jeroen Ploemen <jcfp@debian.org> - 2022-10-27 19:20 +0200
        Re: review for pipenv/2022.10.12-1 Ileana Dumitrescu <ileanadumitrescu95@gmail.com> - 2022-10-28 16:10 +0200
          Re: review for pipenv/2022.10.12-1 Jeroen Ploemen <jcfp@debian.org> - 2022-10-29 09:30 +0200
            Re: review for pipenv/2022.10.12-1 Ileana Dumitrescu <ileanadumitrescu95@gmail.com> - 2022-10-31 15:50 +0100
    Re: review for pipenv/2022.10.12-1 Bastian Venthur <venthur@debian.org> - 2022-10-28 18:20 +0200

#14552 — review for pipenv/2022.10.12-1

[Multipart message — attachments visible in raw view] — view raw

hi Ileana,

I took a look at the package update you prepared and put up for
sponsorship in the Python team:

* leftover boilerplate comments and examples remain throughout the
  packaging (control, rules, watch), please remove when unused.
* changelog: isn't #941447 also fixed by the new release? See
  upstream's comment on https://github.com/pypa/pipenv/issues/4144
* copyright:
  + packaging year bumped for venthur@debian.org but his last
    involvement actually does appear to have been in 2018; you
    probably want to add yourself instead with a 2022 entry?
  + `grep -irn --exclude-dir=debian 'copyr.*\(19\|20\)[0-9]\{2\}' *`
    turns up numerous copyright holders that are missing from
    d/copyright.
* watch: filenamemangle introduces literal "<project>" string into
  the filename.
* lintian:
  + numerous hits for 'extra-license-file' and
    'package-contains-documentation-outside-usr-share-doc', triggered
    by license and readme files inside vendored libs; these files
    could easily be removed during build.
  + E: pipenv: python-traceback-in-manpage is a false positive,
    please override.


PS: I'm kind of surprised a package with this amount of vendoring
managed to survive the ftp masters' review. Apparently, sometimes
miracles do happen.

[toc] | [next] | [standalone]

#14555

Hi,

Thank you for the feedback! I made changes as you suggested. There is
a new upstream version that I also included in the new package. New
updates are in salsa, and I will put a RFS on the IRC channel.

Reading debian package policy I noticed that removing files from a
tarball for a repack (as Bastian suggested in bug #1019714) should
require a +ds suffix, so I packaged the new version with
2022.10.25+ds-1. Please let me know if I did this incorrectly or if
this should not be done for this package.

> + E: pipenv: python-traceback-in-manpage is a false positive,
    please override.

This did not show up in lintian with the new upstream version.

> PS: I'm kind of surprised a package with this amount of vendoring
> managed to survive the ftp masters' review. Apparently, sometimes
> miracles do happen.

I totally agree. This package is quite a mess to deal with. Hopefully
future upstream versions do not keep changing the vendor files,
otherwise the licensing will continue to be a nightmare. I will
continue to monitor pipenv as they are releasing new upstreams quite
frequently.

Ileana Dumitrescu

GPG Public Key: FA26 CA78 4BE1 8892 7F22 B99F 6570 EA01 146F 7354

[toc] | [prev] | [next] | [standalone]

#14559

[Multipart message — attachments visible in raw view] — view raw

On Wed, 26 Oct 2022 17:29:59 +0300
Ileana Dumitrescu <ileanadumitrescu95@gmail.com> wrote:

> Thank you for the feedback! I made changes as you suggested. There
> is a new upstream version that I also included in the new package.

Great! The copyright stuff is a chore on packages like this, so
thanks alot for seeing that through.

> Reading debian package policy I noticed that removing files from a
> tarball for a repack (as Bastian suggested in bug #1019714) should
> require a +ds suffix, so I packaged the new version with
> 2022.10.25+ds-1. Please let me know if I did this incorrectly or if
> this should not be done for this package.

Indeed, a repacksuffix is used to indicate changes were made to an
upstream release so that's perfectly fine this way. Typically, +dfsg
is used to signal the source was repacked for DFSG compliance reasons
and +ds when repacking for some other reason.

I did just notice the upstream release contains several other files
worth considering for removal: a bunch of windows executables [1].
 
> > + E: pipenv: python-traceback-in-manpage is a false positive,  
>     please override.
> 
> This did not show up in lintian with the new upstream version.

It seems they revamped the manpage, although the new one also earns a
lintian hit [1], this time about a bad (missing?) 'whatis' entry.

Lintian seems to think the source for some html file is missing, but
at first glance that hit may well be a false positive triggered by
some bits of javascript.


Unrelated to any of the above, I pushed some minor changes and
enabled the CI on salsa.


[1]https://salsa.debian.org/python-team/packages/pipenv/-/jobs/3434663

[toc] | [prev] | [next] | [standalone]

#14560

> I did just notice the upstream release contains several other files
> worth considering for removal: a bunch of windows executables [1].

I agree and can remove those from the source tarball too. To do that
with the current upstream version in salsa though requires me to git
reset, re-import the 2022.10.25+ds upstream with updated
Files-Excluded, then add back the other commits. I have done that
locally but this requires a force push which is not allowed for the
debian branch since it is protected.

Alternatively I can keep the existing upstream import (which only
excludes get-pipenv.py and not the *.exe files) and add the new
excluded files in debian/copyright for the next upstream import.
Please let me know your preference on these repack options, and I will
update accordingly.

> Lintian seems to think the source for some html file is missing, but
> at first glance that hit may well be a false positive triggered by
> some bits of javascript.

> Unrelated to any of the above, I pushed some minor changes and
> enabled the CI on salsa.

Thanks! I noticed the false positive and since the lintian test on the
pipeline fails, I will add a lintian-overrides file so that it can
pass. I have not pushed that to salsa yet, but I will after hearing
your preference with the repack.

Ileana

[toc] | [prev] | [next] | [standalone]

#14562

[Multipart message — attachments visible in raw view] — view raw

On Fri, 28 Oct 2022 16:46:51 +0300
Ileana Dumitrescu <ileanadumitrescu95@gmail.com> wrote:

> > I did just notice the upstream release contains several other
> > files worth considering for removal: a bunch of windows
> > executables [1].  
> 
> I agree and can remove those from the source tarball too. To do that
> with the current upstream version in salsa though requires me to git
> reset, re-import the 2022.10.25+ds upstream with updated
> Files-Excluded, then add back the other commits. I have done that
> locally but this requires a force push which is not allowed for the
> debian branch since it is protected.

You can avoid resetting or forcing anything by increasing the
repacksuffix. As far as both git and the tooling are concerned, that
makes it an all new upstream version without conflicts with the
repo's current content, so pushing to git works just fine.

First update the excluded files in d/copyright and commit that
change, then run with the usual 'gbp import-orig --pristine-tar
--uscan'. When gbp asks you for the upstream version, modify the +ds
part to +ds2 and proceed with that.

[toc] | [prev] | [next] | [standalone]

#14571

> You can avoid resetting or forcing anything by increasing the
> repacksuffix. As far as both git and the tooling are concerned, that
> makes it an all new upstream version without conflicts with the
> repo's current content, so pushing to git works just fine.

> First update the excluded files in d/copyright and commit that
> change, then run with the usual 'gbp import-orig --pristine-tar
> --uscan'. When gbp asks you for the upstream version, modify the +ds
> part to +ds2 and proceed with that.

Thanks! uscan was not quite letting me use gbp import-orig but I was
able to update the excluded files and have uscan re-download the new
tarball correctly. Then I just had to rename the tarball with the +ds2
version and use gbp import-orig <path to new tarball>. Anyway that
produced the intended result, so I pushed that along with the
lintian-overrides, and the pipeline passes.

>> I totally lost interest in maintaining that package and kind of neglected
>> it because of the vendoring and the package itself or rather its upstream.
>> Anyways, I thought I've orphaned it long time ago (maybe I forgot to do
>> that). So thank you for taking over, I'm sure a lot of users will be happy!

No problem! I am happy to help. I added myself as an uploader also,
and I do not think there is a need to formally orphan if the team
still maintains it (and I will continue to upload).

Ileana

[toc] | [prev] | [next] | [standalone]

#14561

Thanks for taking over!

I totally lost interest in maintaining that package and kind of 
neglected it because of the vendoring and the package itself or rather 
its upstream. Anyways, I thought I've orphaned it long time ago (maybe I 
forgot to do that). So thank you for taking over, I'm sure a lot of 
users will be happy!


Cheers,

Bastian

-- 
Dr. Bastian Venthur                                 https://venthur.de
Debian Developer                                 venthur at debian org

[toc] | [prev] | [standalone]

Back to top | Article view | linux.debian.maint.python

csiph-web