Bug#905866: uscan: prefers watch files from a random dir over debian/watch
Hey Mattia, > Thank you for this, I've now merged it. Thanks! > Also, I personally hate changing long-standing defaults. Yeah, I can see that. > > Maybe the default could be changed to only scan the current directory > > *if* it is a debian source tree, and default to recursive scanning if > > not? That would support both the "Run on a single package" and "Run on a > > collection of packages" usecases neatly? > > That's too surprising. Changing behaviour that way just due to the > surrounding files is too unexpected for me. It might end up doing the right thing in most cases, but I can see it could also be surprising, which is indeed a downside of this approach. > Regardless, I'd accept an MR that would implement: Ok, sounds good. I'd like to submit such a MR, but it's highly likely that I'll not find the time in the near future, or maybe not at all, so if anyone else wants to do this, feel free. > Also perhaps add a relevant config item that would switch the default > locally, so that one can set, e.g., USCAN_RECURSIVE=no in their > ~/.devscripts and have it re-enabled with --recursive as needed. That also sounds like a good idea, that at least allows people to change their own defaults. > > One open question is what constitutes a "source tree" exactly for the > > purpose of the default operation. > > [ -f debian/changelog -a -f debian/watch ] should do IMHO. Without > parsing, it would just fail a few moments later anyway. Given you do not want to change the default as I suggested, I think this question is not actually relevant anymore. Gr. Matthijs signature.asc Description: PGP signature
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
Hi Matthijs! On Tue, May 12, 2020 at 03:22:01PM +0200, Matthijs Kooijman wrote: > Turns out there was a bug with the directory checking. I submitted a fix > here: > > https://salsa.debian.org/debian/devscripts/-/merge_requests/193 > > That MR also has a small change to the manpage that makes it a bit more > explicit that uscan works recursively by default. Thank you for this, I've now merged it. > I still think that the current default might not be ideal. However, I do > see the usecase of running uscan over a collection of debian package at > the same time. Also, I personally hate changing long-standing defaults. > Maybe the default could be changed to only scan the current directory > *if* it is a debian source tree, and default to recursive scanning if > not? That would support both the "Run on a single package" and "Run on a > collection of packages" usecases neatly? That's too surprising. Changing behaviour that way just due to the surrounding files is too unexpected for me. Regardless, I'd accept an MR that would implement: > More specifically, I would suggest: > - Adding a `--no-recursive` option, which will always check only the >current dir (and probably produce an error if no valid package and >watchfile can be found). This might be implemented as an alias for >`--watchfile debian/watch` maybe. this, and > - Adding a `--recursive` option, which ensures that recursive operation >happens, even when the current directory is a valid source tree. This >is what is the current default operation. this, and > - Specifying more than one of `--recursive`, `--no-recursive` and >`--watchfile` is an error. this. > - When none of `--recursive`, `--no-recursive` and `--watchfile` is >specified, the default is to use `--no-recursive` when the current >directory is a source tree, and `--recursive` otherwise. Don't do this, instead leave the default --recursive on. Also perhaps add a relevant config item that would switch the default locally, so that one can set, e.g., USCAN_RECURSIVE=no in their ~/.devscripts and have it re-enabled with --recursive as needed. > One open question is what constitutes a "source tree" exactly for the > purpose of the default operation. The simplest (and most conservative) > is "Whenever a debian/ subdirectory exists", the most extensive is > probably "When a debian/changelog exists and can be parsed and > debian/watch exists". [ -f debian/changelog -a -f debian/watch ] should do IMHO. Without parsing, it would just fail a few moments later anyway. Thank you again for your contribution! :) -- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. More about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `- signature.asc Description: PGP signature
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
Hey folks, > (...) > and then goes on to detail how this directory name checking works > exactly. AFAICS, this directory name checking should protect against > these stray watch files in most of the cases, but apparently it is not > working. > (...) > AFAIU, "subdir" should have matched /^test(-+.)?/, which does not seem > to be the case, so this check seems broken in uscan? Turns out there was a bug with the directory checking. I submitted a fix here: https://salsa.debian.org/debian/devscripts/-/merge_requests/193 That MR also has a small change to the manpage that makes it a bit more explicit that uscan works recursively by default. > - There currently seems to be no way to disable this behaviour at all, >if it turns out to be problematic? I've found that the `--watchfile` option disables recursive processing and just processes the given file instead. So to just process the package in the current directory, you can run `uscan --watchfile debian/watch`. > - The most common usecase seems to be scanning for new versions of a >single package, where this recursive scanning is not needed at all. >Would it not make sense to just scan the current directory by >default, and add an option to enable recursive scanning for usecases >that need it? I still think that the current default might not be ideal. However, I do see the usecase of running uscan over a collection of debian package at the same time. Maybe the default could be changed to only scan the current directory *if* it is a debian source tree, and default to recursive scanning if not? That would support both the "Run on a single package" and "Run on a collection of packages" usecases neatly? More specifically, I would suggest: - Adding a `--no-recursive` option, which will always check only the current dir (and probably produce an error if no valid package and watchfile can be found). This might be implemented as an alias for `--watchfile debian/watch` maybe. - Adding a `--recursive` option, which ensures that recursive operation happens, even when the current directory is a valid source tree. This is what is the current default operation. - Specifying more than one of `--recursive`, `--no-recursive` and `--watchfile` is an error. - When none of `--recursive`, `--no-recursive` and `--watchfile` is specified, the default is to use `--no-recursive` when the current directory is a source tree, and `--recursive` otherwise. One open question is what constitutes a "source tree" exactly for the purpose of the default operation. The simplest (and most conservative) is "Whenever a debian/ subdirectory exists", the most extensive is probably "When a debian/changelog exists and can be parsed and debian/watch exists". I would tend to simplest rather than complex, since that is easier to check and harder to break (e.g. a typo in the changelog causing uscan to behave differently). I *can* imagine that someone would have a debian directory in their package collection they want to run uscan on (e.g. a debian and ubuntu directory for splitting packages between those), so maybe "When debian/changelog exists" is a good compromise here. How does that sound? Gr. Matthijs signature.asc Description: PGP signature
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
Package: devscripts Version: 2.19.4 Followup-For: Bug #905866 Hey folks, I'm also running into problems with uscan processing debian/watch files in subdirectories. Looking at the manpage, it *seems* to say it only scans `debian/watch`, but at the end of the manpage, under ADVANCED FEATURES, it says: uscan actually scans not just the current directory but all its subdirectories looking for debian/watch to process them all. See "Directory name checking". Looking at the section "Directory name checking", it says: Similarly to several other scripts in the devscripts package, uscan explores the requested directory trees looking for debian/changelog and debian/watch files. As a safeguard against stray files causing potential problems, and in order to promote efficiency, it will examine the name of the parent directory once it finds the debian/changelog file, and check that the directory name corresponds to the package name. It will only attempt to download newer versions of the package and then perform any requested action if the directory name matches the package name. and then goes on to detail how this directory name checking works exactly. AFAICS, this directory name checking should protect against these stray watch files in most of the cases, but apparently it is not working. To reproduce: $ mkdir -p subdir/debian $ (cd subdir; dch --create --package test --newversion 1.0.0-1) $ touch subdir/debian/watch $ uscan --report $ uscan --report-status uscan info: uscan (version 2.19.4) See uscan(1) for help uscan info: Scan watch files in . uscan info: Check debian/watch and debian/changelog in ./subdir uscan info: package="test" version="1.0.0-1" (as seen in debian/changelog) uscan info: package="test" version="1.0.0" (no epoch/revision) uscan info: ./subdir/debian/changelog sets package="test" version="1.0.0" uscan info: Process watch file at: debian/watch package = test version = 1.0.0 pkg_dir = ./subdir uscan info: Scan finished AFAIU, "subdir" should have matched /^test(-+.)?/, which does not seem to be the case, so this check seems broken in uscan? However, I'm actually wondering if this recursive scanning by default is a good feature at all. Two considerations: - There currently seems to be no way to disable this behaviour at all, if it turns out to be problematic? - The most common usecase seems to be scanning for new versions of a single package, where this recursive scanning is not needed at all. Would it not make sense to just scan the current directory by default, and add an option to enable recursive scanning for usecases that need it? Gr. Matthijs
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
On Thu, Apr 11, 2019 at 10:40:54AM -0700, Daniel Schepler wrote:
> I find the recursive search feature useful for when I have a large
> collection of unpacked source packages in $HOME/src/debian, then I can
> "cd ~/src/debian; uscan --report" to check for updates to any of those
> source packages.
>
> As for an example I've run into:
> https://ftp.gnu.org/gnu/ncurses/ncurses-6.1.tar.gz contains several
> invalid debian/watch files which trigger error messages in the
> multiple-package uscan run until I manually delete them.
Hmm, I'm not sure which side you're arguing for then. The former sounds
as if you prefer recursive search, while the latter says why it's bad.
My issue with it is that:
* there's no easy way to get rid of a bogus watch file (would need to
repack the upstream tarball)
* our official tools (such as the QA tracker) can't cope with known-bad
output
Even the use case you mention would work better with a shell one-liner
such as:
for x in */debian/watch;do uscan --report "${x%/debian/watch}";done
which won't fall into the bad watch files.
Meow!
--
⢀⣴⠾⠻⢶⣦⠀
⣾⠁⢠⠒⠀⣿⡁ Did ya know that typing "test -j8" instead of "ctest -j8"
⢿⡄⠘⠷⠚⠋⠀ will make your testsuite pass much faster, and fix bugs?
⠈⠳⣄
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
I find the recursive search feature useful for when I have a large collection of unpacked source packages in $HOME/src/debian, then I can "cd ~/src/debian; uscan --report" to check for updates to any of those source packages. As for an example I've run into: https://ftp.gnu.org/gnu/ncurses/ncurses-6.1.tar.gz contains several invalid debian/watch files which trigger error messages in the multiple-package uscan run until I manually delete them. -- Daniel Schepler
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
Hi, could you give me an example ? Cheers, Xavier
Bug#905866: uscan: prefers watch files from a random dir over debian/watch
Package: devscripts Version: 2.18.3 Severity: normal Hi! If the upstream source includes some bad debian packaging, it's likely there'll be a watch file somewhere, usually bogus and/or stale. For this reason, source format 3.0 deletes /debian/ from inside the tarball -- and if the upstream packaging lives in a non-standard places, that's harmless as no tool has bogus ideas like searching elsewhere. Other than uscan, that is. If there's a bad watch file, let's say source/debian/watch, uscan will randomly (based on filesystem order?) prefer that over debian/watch. Source format 3.0 (quilt) doesn't allow deleting files sanely: it ignores deletions, assuming that the clean target will delete them before build. This works for anything that calls debian/rules targets, but not for uscan. Thus, I'd need to repack the .orig tarball just to workaround this. This is similar to #895982 (uscan looking for debian/changelog in VCS) dirs. What's even the purpose of search for watch and changelog in places other than debian/ ? Meow. -- Package-specific info: --- /etc/devscripts.conf --- --- ~/.devscripts --- DEBCHANGE_RELEASE_HEURISTIC=log DEBSIGN_KEYID=FD9CE2D8D7754B78AB279BBD2C3B436FEAC68101 DSCVERIFY_KEYRINGS="~/.gnupg/pubring.gpg" -- System Information: Debian Release: buster/sid APT prefers unstable-debug APT policy: (500, 'unstable-debug'), (500, 'unstable'), (500, 'testing'), (150, 'experimental') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 4.18.0-rc7-debug-00037-gda205b923a99 (SMP w/6 CPU cores) Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE=C.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: sysvinit (via /sbin/init)
