NetBSDfr / pkgin

pkgin's official github repository
130 stars 40 forks source link

Fix #114, expand `pkgargs` in order to match globs #117

Closed iMilnb closed 3 years ago

iMilnb commented 3 years ago

This fixes #114 totally. In both pkgin in and pkgin rm, globs were not expanded, when installing, this forbids a package requested as a glob (i.e. s*ege) to be recorded as "keepable". They were also not expanded when deleting a package, meaning pkgin rm s*ege would not find any package to process.
The package list is now modifiable and expanded for further processing.

jperkin commented 3 years ago

Thanks for looking at this! I have a few comments:

$ pkgin search tiny*db
No results found for tiny*db
$ pkgin search tiny.*db
tinycdb-0.78         Create and read constant databases

$ pkgin install tiny.*db
tiny.*db is not available in the repository
$ pkgin install tiny*db
calculating dependencies...done.

1 package to install:
  tinycdb-0.78

Not something we should fix as part of this commit but definitely something that needs fixing at some point.

$ pkgin rm tiny*db
1 packages to delete: 
  tinycdb-0.78

proceed ? [Y/n] 
removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6 (core dumped)
(lldb) thread backtrace 
* thread #1, stop reason = signal SIGSTOP
  * frame #0: 0x00007fff202e492e libsystem_kernel.dylib`__pthread_kill + 10
    frame #1: 0x00007fff203135bd libsystem_pthread.dylib`pthread_kill + 263
    frame #2: 0x00007fff20268411 libsystem_c.dylib`abort + 120
    frame #3: 0x00007fff201481f5 libsystem_malloc.dylib`malloc_vreport + 548
    frame #4: 0x00007fff2014b34a libsystem_malloc.dylib`malloc_report + 151
    frame #5: 0x000000010ec800f9 pkgin`free_pkglist_entry(plist=0x00007ffee0f8a1b0) at pkglist.c:96:2
    frame #6: 0x000000010ec80481 pkgin`free_global_pkglist(plisthead=0x000000010ec9a050) at pkglist.c:159:3
    frame #7: 0x000000010ec8042c pkgin`free_global_pkglists at pkglist.c:167:2
    frame #8: 0x000000010ec7d0c6 pkgin`main(argc=2, argv=0x00007ffee0f8a270) at main.c:308:2
    frame #9: 0x00007fff2032ef5d libdyld.dylib`start + 1
    frame #10: 0x00007fff2032ef5d libdyld.dylib`start + 1
(lldb) frame select 5
frame #5: 0x000000010ec800f9 pkgin`free_pkglist_entry(plist=0x00007ffee0f8a1b0) at pkglist.c:96:2
   93   free_pkglist_entry(Pkglist **plist)
   94   {
   95       XFREE((*plist)->full);
-> 96       XFREE((*plist)->name);
   97       XFREE((*plist)->version);
   98       XFREE((*plist)->depend);
   99       XFREE((*plist)->comment);
(lldb) print (*plist)->full
(char *) $9 = 0x0000000000000000
(lldb) print (*plist)->name
(char *) $10 = 0x00007fcd81ec5890 "\xff\xff\U0000001e\xd8\xfc\a"
(lldb) print (*plist)->version
(char *) $11 = 0x00007fcd81ec58a0 "2.7.4nb11"
(lldb) print (*plist)->depend
(char *) $12 = 0x0000000000000000
(lldb) print (*plist)->comment
(char *) $13 = 0x00007fcd81ec55c0 "Network UPS Tools perl binding"

(*plist)->name looks like garbage to me, and the rest doesn't match the package we're operating on but is coming from the unrelated sysutils/p5-UPS-Nut. We should also fix the incorrect plural "1 packages to delete".

iMilnb commented 3 years ago
  • I'd prefer to keep the autoconf changes to a separate commit, and would also like to just hold off on these for a while. We ran into a number of issues in pkgsrc after the bump from 2.69 and I'd like to test it a bit more thoroughly.

done, I've reverted to autoconf 2.69 generated files.

  • Unrelated to this fix, but it's confusing that pkgin search takes regular expressions whereas pkgin install/remove etc take globs:

I agree, the main reason is that package matching for installation / removal uses opattern.c from pkg_install which I'd prefer not to mess with ;)

  • While the test suite runs clean against this change, I see an abort when testing manually on macOS (we don't yet have any tests for glob stuff in the test suite):
$ pkgin rm tiny*db
1 packages to delete: 
  tinycdb-0.78

proceed ? [Y/n] 
removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6 (core dumped)

That's interesting... does it only fails with a glob? or does it also happen with a regular name?

jperkin commented 3 years ago

Ah good question, it fails without a glob too:

$ sudo ./pkgin rm tinycdb
1 packages to delete: 
  tinycdb-0.78

proceed ? [Y/n] 
removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6
iMilnb commented 3 years ago

Ah good question, it fails without a glob too:

$ sudo ./pkgin rm tinycdb
1 packages to delete: 
  tinycdb-0.78

proceed ? [Y/n] 
removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6

Does this behavior appeared with the #114 fix? (sorry I can't reproduce it anywhere)

jperkin commented 3 years ago

Yes, only happens with the patch. I've seen similar problems on macOS before, it's more strict about uninitialised memory etc.

iMilnb commented 3 years ago

Yes, only happens with the patch. I've seen similar problems on macOS before, it's more strict about uninitialised memory etc.

I managed to have an access on a Mojave/18.7.0 OSX machine, and can't reproduce this behavior neither. Can you share a bit more about the environment you use?

jperkin commented 3 years ago

Sure:

$ sw_vers 
ProductName:    macOS
ProductVersion: 11.4
BuildVersion:   20F71

$ clang -v
Apple clang version 12.0.5 (clang-1205.0.22.9)
Target: x86_64-apple-darwin20.5.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

$ git show --stat
commit 48e3bc89e20f63c3a0b11d44238a6212d9f30125 (HEAD -> imil)
Author: Emile 'iMil' Heitor <imil@NetBSD.org>
Date:   Sun Jul 18 12:03:57 2021 +0200

    fix: reverted to autoconf 2.69 as per https://github.com/NetBSDfr/pkgin/pull/117#issuecomment-882018400

 Makefile.in |    4 +-
 config.h.in |   10 +-
 configure   | 3229 +++++++++++++++++++++++++++++++++++++++++++-----------------------------------------------
 3 files changed, 1541 insertions(+), 1702 deletions(-)

$ ./config.status --config
'--prefix=/opt/pkg' '--with-dbdir=/var/db/pkgin' '--with-machine-arch=x86_64' '--with-libarchive=/opt/pkg' '--with-openssl=/opt/pkg' '--with-sqlite3=/opt/pkg' '--with-libfetch=/opt/pkg' '--with-pkg-install=/opt/pkg/sbin'

$ make clean; make
test -z "pkgin" || rm -f pkgin
test -z "pkgin.1 pkgindb_create.h" || rm -f pkgin.1 pkgindb_create.h
rm -f *.o
rm -f external/*.o
/Library/Developer/CommandLineTools/usr/bin/make  all-am
  CC       pkgin-actions.o
  CC       pkgin-autoremove.o
  CC       pkgin-depends.o
  CC       pkgin-download.o
  CC       pkgin-fsops.o
  CC       pkgin-impact.o
  CC       pkgin-main.o
  CC       pkgin-order.o
  CC       pkgin-pkg_check.o
  CC       pkgin-pkg_infos.o
  CC       pkgin-pkg_install.o
  CC       pkgin-pkg_str.o
  CC       pkgin-pkgindb.o
  CC       pkgin-pkgindb_queries.o
  CC       pkgin-pkglist.o
  CC       pkgin-preferred.o
  CC       pkgin-selection.o
  CC       pkgin-sqlite_callbacks.o
  CC       pkgin-summary.o
  CC       pkgin-tools.o
  CC       external/pkgin-progressmeter.o
  CC       external/pkgin-automatic.o
  CC       external/pkgin-dewey.o
  CC       external/pkgin-fexec.o
  CC       external/pkgin-opattern.o
  CC       external/pkgin-pkgdb.o
  CC       external/pkgin-var.o
  CC       external/pkgin-xwrapper.o
  CCLD     pkgin

Patched pkgin, SIGABRT at end:

$ ./pkgin -v
pkgin 20.12.1 (using SQLite 3.36.0)

$ sudo ./pkgin up
processing remote summary (https://pkgsrc.joyent.com/packages/Darwin/11.0/x86_64/All)...
database for https://pkgsrc.joyent.com/packages/Darwin/11.0/x86_64/All is up-to-date

$ sudo ./pkgin -y in tinycdb
calculating dependencies...done.

1 package to install:
  tinycdb-0.78

0 to refresh, 0 to upgrade, 1 to install
0B to download, 200K to install

installing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
marking tinycdb-0.78 as non auto-removable

$ sudo ./pkgin -y rm tinycdb
1 packages to delete: 
  tinycdb-0.78

removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6

Unpatched system version, works fine:

$ type pkgin
pkgin is /opt/pkg/bin/pkgin

$ pkgin -v
pkgin 20.12.1 (using SQLite 3.36.0)

$ sudo pkgin -y in tinycdb
calculating dependencies...done.

1 package to install:
  tinycdb-0.78

0 to refresh, 0 to upgrade, 1 to install
0B to download, 200K to install

installing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
marking tinycdb-0.78 as non auto-removable

$ sudo pkgin -y rm tinycdb
1 packages to delete: 
  tinycdb-0.78

removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...

Revert patched version to HEAD, works fine:

$ git checkout origin/HEAD

$ git show --stat
commit 56d4cdd21cbe903cc6b7fb35873ad19260275b93 (HEAD, origin/master, origin/HEAD)
Merge: e963d1e 121dee2
Author: iMil <imil@NetBSD.org>
Date:   Sat Jul 3 11:34:26 2021 +0200

    Merge pull request #115 from ralfdoering/patch-1

    Change IRC network to Libera in index.html

 htdocs/index.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

$ ./config.status --config
'--prefix=/opt/pkg' '--with-dbdir=/var/db/pkgin' '--with-machine-arch=x86_64' '--with-libarchive=/opt/pkg' '--with-openssl=/opt/pkg' '--with-sqlite3=/opt/pkg' '--with-libfetch=/opt/pkg' '--with-pkg-install=/opt/pkg/sbin'

$ make clean; make
test -z "pkgin" || rm -f pkgin
test -z "pkgin.1 pkgindb_create.h" || rm -f pkgin.1 pkgindb_create.h
rm -f *.o
rm -f external/*.o
/Library/Developer/CommandLineTools/usr/bin/make  all-am
  CC       pkgin-actions.o
  CC       pkgin-autoremove.o
  CC       pkgin-depends.o
  CC       pkgin-download.o
  CC       pkgin-fsops.o
  CC       pkgin-impact.o
  CC       pkgin-main.o
  CC       pkgin-order.o
  CC       pkgin-pkg_check.o
  CC       pkgin-pkg_infos.o
  CC       pkgin-pkg_install.o
  CC       pkgin-pkg_str.o
  CC       pkgin-pkgindb.o
  CC       pkgin-pkgindb_queries.o
  CC       pkgin-pkglist.o
  CC       pkgin-preferred.o
  CC       pkgin-selection.o
  CC       pkgin-sqlite_callbacks.o
  CC       pkgin-summary.o
  CC       pkgin-tools.o
  CC       external/pkgin-progressmeter.o
  CC       external/pkgin-automatic.o
  CC       external/pkgin-dewey.o
  CC       external/pkgin-fexec.o
  CC       external/pkgin-opattern.o
  CC       external/pkgin-pkgdb.o
  CC       external/pkgin-var.o
  CC       external/pkgin-xwrapper.o
  CCLD     pkgin

$ sudo ./pkgin -y in tinycdb
calculating dependencies...done.

1 package to install:
  tinycdb-0.78

0 to refresh, 0 to upgrade, 1 to install
0B to download, 200K to install

installing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
marking tinycdb-0.78 as non auto-removable

$ sudo ./pkgin -y rm tinycdb
1 packages to delete: 
  tinycdb-0.78

removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
jperkin commented 3 years ago

FWIW I can reproduce this on Mojave too:

$ sw_vers
ProductName:    Mac OS X
ProductVersion: 10.14.6
BuildVersion:   18G9216

$ clang -v
Apple LLVM version 10.0.1 (clang-1001.0.46.4)
Target: x86_64-apple-darwin18.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

$ cd pkgsrc/pkgtools/pkgin

$ git diff -U0 Makefile 
diff --git a/pkgtools/pkgin/Makefile b/pkgtools/pkgin/Makefile
index c2dfb92b1ce..dce4071c5ee 100644
--- a/pkgtools/pkgin/Makefile
+++ b/pkgtools/pkgin/Makefile
@@ -7 +7 @@ MASTER_SITES=               ${MASTER_SITE_GITHUB:=NetBSDfr/}
-GITHUB_TAG=            v${PKGVERSION_NOREV}
+GITHUB_TAG=            48e3bc89e20f63c3a0b11d44238a6212d9f30125

$ bmake mdi; bmake
[...]

$ cd $(bmake show-var VARNAME=WRKSRC)

$ export PKG_REPOS=https://pkgsrc.joyent.com/packages/Darwin/trunk/x86_64/All

$ ./pkgin -y in tinycdb
reading local summary...
processing local summary...
processing remote summary (https://pkgsrc.joyent.com/packages/Darwin/trunk/x86_64/All)...
pkg_summary.xz                                                                    100% 2165KB 721.6KB/s   00:03    
calculating dependencies...done.

1 package to install:
  tinycdb-0.78

0 to refresh, 0 to upgrade, 1 to install
37K to download, 96K to install

tinycdb-0.78.tgz                                                                  100%   37KB  36.9KB/s   00:00    
installing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
marking tinycdb-0.78 as non auto-removable

$ ./pkgin -y rm tinycdb
1 packages to delete: 
  tinycdb-0.78

removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6
iMilnb commented 3 years ago

FWIW I can reproduce this on Mojave too:

yup, I managed to reproduce it too, I can work on it now, thanks!

iMilnb commented 3 years ago
$ ./pkgin -y rm tinycdb
1 packages to delete: 
  tinycdb-0.78

removing tinycdb-0.78...
pkg_install warnings: 0, errors: 0
reading local summary...
processing local summary...
Abort trap: 6

@jperkin can you try the latest commit please?