GNU bug report logs - #62375
[PATCH 0/1] npm binary importer

Previous Next

Full log

Message #52 received at 62375 <at> debbugs.gnu.org (full text, mbox):

From: Jelle Licht <jlicht <at> fsfe.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: Timothy Sample <samplet <at> ngyro.com>, Josselin Poiret <dev <at> jpoiret.xyz>,
 Simon Tournier <zimon.toutoune <at> gmail.com>, Mathieu Othacehe <othacehe <at> gnu.org>,
 Tobias Geerinckx-Rice <me <at> tobias.gr>, Lars-Dominik Braun <lars <at> 6xq.net>,
 62375 <at> debbugs.gnu.org, Christopher Baines <guix <at> cbaines.net>,
 Ricardo Wurmus <rekado <at> elephly.net>
Subject: Re: [bug#62375] [PATCH v3] import: Add binary npm importer.
Date: Tue, 02 Apr 2024 16:12:01 +0200

Hi Ludo',

Thanks for the speedy review!

Ludovic Courtès <ludo <at> gnu.org> writes:

> Hi!
>
> jlicht <at> fsfe.org skribis:
>
>> From: Jelle Licht <jlicht <at> fsfe.org>
>>
>> * guix/scripts/import.scm: (importers): Add "npm-binary".
>> * guix/import/npm-binary.scm: New file.
>> * guix/scripts/import/npm-binary.scm: New file.
>> * Makefile.am: Add them.
>>
>> Co-authored-by: Timothy Sample <samplet <at> ngyro.com>
>> Co-authored-by: Lars-Dominik Braun <lars <at> 6xq.net>
>>
>> Change-Id: I98a45068cf5b9c42790664cc743feaa7ac76f807
>
> Yay!
>
>> +The npm-binary importer also allows you to specify a version string:
>> +
>> +@example
>> +guix import npm-binary buffer-crc32 1.0.0
>> +@end example
>
> For consistency with other importers (pypi, gem, cran), could you change
> the syntax to:
>
>   guix import npm-binary buffer-crc32 <at> 1.0.0
>
> ?
>
> That’s the last remaining issue for me.
>

I needed some custom logic to support the npm scoped packages
("@linthtml/linthtml <at> 1.2.3"), but it should now work.

>> +;; TODO: Support other registries
>> +(define %registry "https://registry.npmjs.org")
>
> For the purposes of tests, you could make it:
>
>   (define %npm-registry
>     (make-parameter "https://registry.npmjs.org"))

Easy!

> That would allow you to write tests using ‘with-http-server’ and
> ‘parameterize’ as done in ‘tests/pypi.scm’ and others, which I find
> nicer and more robust than ‘mock’.

The `with-http-server' construct does not play well with an interactive
REPL workflow due to not cleaning up the used port in all situations,
making that port unavailable for running the test again[1]. `mock',
brittle as it may be, does not to suffer from this drawback, so I'd
vastly prefer using that for now.

> Not a blocker though.
>
> Thanks!
>
> Ludo’.

I'll send a v4, let it simmer for some days and then merge it if no big
blockers or requested changes show up.

- Jelle

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.