* [bug#38754] [PATCH 0/2] Speed up the derivation linter.
@ 2019-12-26 17:33 Christopher Baines
2019-12-26 18:01 ` [bug#38754] [PATCH 1/2] guix: lint: Add an optional parameter for a store connection Christopher Baines
2019-12-30 22:01 ` [bug#38754] [PATCH 0/2] Speed up the derivation linter Ludovic Courtès
0 siblings, 2 replies; 14+ messages in thread
From: Christopher Baines @ 2019-12-26 17:33 UTC (permalink / raw)
To: 38754
[-- Attachment #1: Type: text/plain, Size: 595 bytes --]
These patches speed up the derivation linter by reducing the number of
times a connection to the guix-daemon is established. In testing on my
computer, the time to lint all guix packages with the derivation linter
drops from around 6 minutes to around 3 minutes.
Christopher Baines (2):
guix: lint: Add an optional parameter for a store connection.
scripts: lint: Set the %link-checker-store-connection parameter.
guix/lint.scm | 42 ++++++++++++++++++++++++++----------------
guix/scripts/lint.scm | 22 +++++++++++++---------
2 files changed, 39 insertions(+), 25 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 1/2] guix: lint: Add an optional parameter for a store connection.
2019-12-26 17:33 [bug#38754] [PATCH 0/2] Speed up the derivation linter Christopher Baines
@ 2019-12-26 18:01 ` Christopher Baines
2019-12-26 18:01 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
2019-12-30 22:01 ` [bug#38754] [PATCH 0/2] Speed up the derivation linter Ludovic Courtès
1 sibling, 1 reply; 14+ messages in thread
From: Christopher Baines @ 2019-12-26 18:01 UTC (permalink / raw)
To: 38754
Previously, the derivation lint checker establishes a connection to the store
for each supported system of each package. This change uses the same store
connection for all supported systems, with the option of setting a parameter
for a store connection which will be used instead of establishing a new
connection.
Previously, running the derivation linter for all packages would take around 6
and a half minutes, with this change, without setting the
%lint-checker-store-connection parameter, the time is reduced to around 4
minutes.
* guix/lint.scm (%lint-checker-store-connection): New parameter.
(check-derivation): Arrange the code so that it's possible to either run with
the store from the new parameter, or open a new connection via the with-store
syntax.
---
guix/lint.scm | 42 ++++++++++++++++++++++++++----------------
1 file changed, 26 insertions(+), 16 deletions(-)
diff --git a/guix/lint.scm b/guix/lint.scm
index cd2ea571ed..19498db857 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -100,7 +100,9 @@
lint-checker?
lint-checker-name
lint-checker-description
- lint-checker-check))
+ lint-checker-check
+
+ %lint-checker-store-connection))
\f
;;;
@@ -142,6 +144,9 @@
((_ package (G_ message) rest ...)
(%make-warning package message rest ...))))
+(define %lint-checker-store-connection
+ (make-parameter #f))
+
\f
;;;
;;; Checkers
@@ -887,7 +892,7 @@ descriptions maintained upstream."
(define (check-derivation package)
"Emit a warning if we fail to compile PACKAGE to a derivation."
- (define (try system)
+ (define (try store system)
(catch #t
(lambda ()
(guard (c ((store-protocol-error? c)
@@ -900,25 +905,30 @@ descriptions maintained upstream."
(G_ "failed to create ~a derivation: ~a")
(list system
(condition-message c)))))
- (with-store store
- ;; Disable grafts since it can entail rebuilds.
- (parameterize ((%graft? #f))
- (package-derivation store package system #:graft? #f)
-
- ;; If there's a replacement, make sure we can compute its
- ;; derivation.
- (match (package-replacement package)
- (#f #t)
- (replacement
- (package-derivation store replacement system
- #:graft? #f)))))))
+ ;; Disable grafts since it can entail rebuilds.
+ (parameterize ((%graft? #f))
+ (package-derivation store package system #:graft? #f)
+
+ ;; If there's a replacement, make sure we can compute its
+ ;; derivation.
+ (match (package-replacement package)
+ (#f #t)
+ (replacement
+ (package-derivation store replacement system
+ #:graft? #f))))))
(lambda args
(make-warning package
(G_ "failed to create ~a derivation: ~s")
(list system args)))))
- (filter lint-warning?
- (map try (package-supported-systems package))))
+ (define (check-with-store store)
+ (filter lint-warning?
+ (map (cut try store <>) (package-supported-systems package))))
+
+ (or (and=> (%lint-checker-store-connection)
+ check-with-store)
+ (with-store store
+ (check-with-store store))))
(define (check-license package)
"Warn about type errors of the 'license' field of PACKAGE."
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2019-12-26 18:01 ` [bug#38754] [PATCH 1/2] guix: lint: Add an optional parameter for a store connection Christopher Baines
@ 2019-12-26 18:01 ` Christopher Baines
2019-12-30 22:12 ` Ludovic Courtès
0 siblings, 1 reply; 14+ messages in thread
From: Christopher Baines @ 2019-12-26 18:01 UTC (permalink / raw)
To: 38754
If set, this parameter provides a store connection used by the derivation
linter. Without this being set, the derivation linter establishes a new
connection for each package. With this change, I saw the time taken to lint
all packages with the derivation linter drop from over 4 minutes to around 3
minutes.
* guix/scripts/lint.scm (guix-lint): Set the %lint-checker-store-connection)
parameter.
---
guix/scripts/lint.scm | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 8d08c484f5..47c104217d 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -28,6 +28,7 @@
(define-module (guix scripts lint)
#:use-module (guix packages)
+ #:use-module (guix store)
#:use-module (guix lint)
#:use-module (guix ui)
#:use-module (guix scripts)
@@ -167,12 +168,15 @@ run the checkers on all packages.\n"))
(_ #f))
(reverse opts)))
(checkers (or (assoc-ref opts 'checkers) %all-checkers)))
- (cond
- ((assoc-ref opts 'list?)
- (list-checkers-and-exit checkers))
- ((null? args)
- (fold-packages (lambda (p r) (run-checkers p checkers)) '()))
- (else
- (for-each (lambda (spec)
- (run-checkers (specification->package spec) checkers))
- args)))))
+ (with-store store
+ (parameterize
+ ((%lint-checker-store-connection store))
+ (cond
+ ((assoc-ref opts 'list?)
+ (list-checkers-and-exit checkers))
+ ((null? args)
+ (fold-packages (lambda (p r) (run-checkers p checkers)) '()))
+ (else
+ (for-each (lambda (spec)
+ (run-checkers (specification->package spec) checkers))
+ args)))))))
--
2.24.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 0/2] Speed up the derivation linter.
2019-12-26 17:33 [bug#38754] [PATCH 0/2] Speed up the derivation linter Christopher Baines
2019-12-26 18:01 ` [bug#38754] [PATCH 1/2] guix: lint: Add an optional parameter for a store connection Christopher Baines
@ 2019-12-30 22:01 ` Ludovic Courtès
1 sibling, 0 replies; 14+ messages in thread
From: Ludovic Courtès @ 2019-12-30 22:01 UTC (permalink / raw)
To: Christopher Baines; +Cc: 38754
Hi,
Christopher Baines <mail@cbaines.net> skribis:
> These patches speed up the derivation linter by reducing the number of
> times a connection to the guix-daemon is established. In testing on my
> computer, the time to lint all guix packages with the derivation linter
> drops from around 6 minutes to around 3 minutes.
Neat.
> Christopher Baines (2):
> guix: lint: Add an optional parameter for a store connection.
> scripts: lint: Set the %link-checker-store-connection parameter.
LGTM, thanks! :-)
Ludo’.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2019-12-26 18:01 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
@ 2019-12-30 22:12 ` Ludovic Courtès
2019-12-30 23:34 ` Christopher Baines
0 siblings, 1 reply; 14+ messages in thread
From: Ludovic Courtès @ 2019-12-30 22:12 UTC (permalink / raw)
To: Christopher Baines; +Cc: 38754
Christopher Baines <mail@cbaines.net> skribis:
> + (with-store store
> + (parameterize
> + ((%lint-checker-store-connection store))
Actually it means that now ‘guix lint’ systematically connects to the
daemon.
I wonder if we could arrange to open the connection lazily, and to
somehow carry state across linter invocations. Perhaps
‘check-derivation’ should be monadic, with a field in <checker>
indicating that. Sounds complicated though.
Thoughts?
Ludo’.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2019-12-30 22:12 ` Ludovic Courtès
@ 2019-12-30 23:34 ` Christopher Baines
2019-12-31 18:15 ` Ludovic Courtès
0 siblings, 1 reply; 14+ messages in thread
From: Christopher Baines @ 2019-12-30 23:34 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 38754
[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]
Ludovic Courtès <ludo@gnu.org> writes:
> Christopher Baines <mail@cbaines.net> skribis:
>
>> + (with-store store
>> + (parameterize
>> + ((%lint-checker-store-connection store))
>
> Actually it means that now ‘guix lint’ systematically connects to the
> daemon.
I guess that's the effect, were you meaning this would make a better
message in the commit?
> I wonder if we could arrange to open the connection lazily, and to
> somehow carry state across linter invocations. Perhaps
> ‘check-derivation’ should be monadic, with a field in <checker>
> indicating that. Sounds complicated though.
>
> Thoughts?
I did wonder if the code could somehow transparently be made more
efficient. Quite often database clients manage a pool of connections,
and when you perform a database operation, a connection from the pool is
checked out, and then returned once you're finished. But as you say,
this could be complicated. I think parameters can be set with
connections, and I'm not quiet sure what the interface should be.
I also did think about somehow passing the store connection in to the
lint checker more explicitly, but I'm not sure how to generalise that.
At least to me, a parameter to store the connection in seemed like a
simple if inelegant approach. I'm quite happy to look in to other
approaches though if you have thoughts on what might be good.
Thanks,
Chris
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2019-12-30 23:34 ` Christopher Baines
@ 2019-12-31 18:15 ` Ludovic Courtès
2020-03-15 21:06 ` [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record Christopher Baines
2020-03-15 21:35 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
0 siblings, 2 replies; 14+ messages in thread
From: Ludovic Courtès @ 2019-12-31 18:15 UTC (permalink / raw)
To: Christopher Baines; +Cc: 38754
Hi Chris!
Christopher Baines <mail@cbaines.net> skribis:
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Christopher Baines <mail@cbaines.net> skribis:
>>
>>> + (with-store store
>>> + (parameterize
>>> + ((%lint-checker-store-connection store))
>>
>> Actually it means that now ‘guix lint’ systematically connects to the
>> daemon.
>
> I guess that's the effect, were you meaning this would make a better
> message in the commit?
I mean that it’s a visible change. Before, you could run all the
linters but this one without having a daemon running; now you need a
daemon up and running.
>> I wonder if we could arrange to open the connection lazily, and to
>> somehow carry state across linter invocations. Perhaps
>> ‘check-derivation’ should be monadic, with a field in <checker>
>> indicating that. Sounds complicated though.
>>
>> Thoughts?
>
> I did wonder if the code could somehow transparently be made more
> efficient. Quite often database clients manage a pool of connections,
> and when you perform a database operation, a connection from the pool is
> checked out, and then returned once you're finished. But as you say,
> this could be complicated. I think parameters can be set with
> connections, and I'm not quiet sure what the interface should be.
>
> I also did think about somehow passing the store connection in to the
> lint checker more explicitly, but I'm not sure how to generalise that.
There could be a <checker> field indicating either that (1) the
procedure takes an optional store parameter, or that (2) the procedure
is monadic in ‘%store-monad’.
#2 seems more complicated to implement that #1 though.
For #1, ‘guix lint’ could check whether:
(any checker-require-store? checkers)
is true, and if it is, it could open a connection and pass it on as
needed.
WDYT?
If that seems good to you, I guess you can go ahead with it (let’s just
not lose our hair on it!).
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record.
2019-12-31 18:15 ` Ludovic Courtès
@ 2020-03-15 21:06 ` Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 2/4] lint: Mark the derivation checker as requiring a store connection Christopher Baines
` (2 more replies)
2020-03-15 21:35 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
1 sibling, 3 replies; 14+ messages in thread
From: Christopher Baines @ 2020-03-15 21:06 UTC (permalink / raw)
To: 38754
This can then be used to mark checkers that require a store connection, which
will enable passing a connection in, avoiding the overhead of establishing a
connection inside the check function when it's run for lots of different
packages.
* guix/lint.scm (<lint-checker>): Add requires-store? to the record type.
---
guix/lint.scm | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/guix/lint.scm b/guix/lint.scm
index 24fbf05202..2a084382c6 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -100,7 +100,8 @@
lint-checker?
lint-checker-name
lint-checker-description
- lint-checker-check))
+ lint-checker-check
+ lint-checker-requires-store?))
\f
;;;
@@ -155,7 +156,9 @@
;; 'certainty' level.
(name lint-checker-name)
(description lint-checker-description)
- (check lint-checker-check))
+ (check lint-checker-check)
+ (requires-store? lint-checker-requires-store?
+ (default #f)))
(define (properly-starts-sentence? s)
(string-match "^[(\"'`[:upper:][:digit:]]" s))
--
2.25.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/4] lint: Mark the derivation checker as requiring a store connection.
2020-03-15 21:06 ` [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record Christopher Baines
@ 2020-03-15 21:06 ` Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 3/4] lint: Add a #:store argument to check-derivation Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 4/4] scripts: lint: Handle store connections for lint checkers Christopher Baines
2 siblings, 0 replies; 14+ messages in thread
From: Christopher Baines @ 2020-03-15 21:06 UTC (permalink / raw)
To: 38754
* guix/lint.scm (%local-checkers): Mark the derivation checker as requiring a
store connection.
---
guix/lint.scm | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/guix/lint.scm b/guix/lint.scm
index 2a084382c6..b20510b45d 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -1330,9 +1330,10 @@ or a list thereof")
(description "Check for autogenerated tarballs")
(check check-source-unstable-tarball))
(lint-checker
- (name 'derivation)
- (description "Report failure to compile a package to a derivation")
- (check check-derivation))
+ (name 'derivation)
+ (description "Report failure to compile a package to a derivation")
+ (check check-derivation)
+ (requires-store? #t))
(lint-checker
(name 'patch-file-names)
(description "Validate file names and availability of patches")
--
2.25.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 3/4] lint: Add a #:store argument to check-derivation
2020-03-15 21:06 ` [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 2/4] lint: Mark the derivation checker as requiring a store connection Christopher Baines
@ 2020-03-15 21:06 ` Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 4/4] scripts: lint: Handle store connections for lint checkers Christopher Baines
2 siblings, 0 replies; 14+ messages in thread
From: Christopher Baines @ 2020-03-15 21:06 UTC (permalink / raw)
To: 38754
This can then be used to avoid opening up a store connection each time a
package needs checking.
* guix/lint.scm (check-derivation): Add a #:store argument, and pull the
handling of the store connection out of the try function.
---
guix/lint.scm | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/guix/lint.scm b/guix/lint.scm
index b20510b45d..cfe3be2302 100644
--- a/guix/lint.scm
+++ b/guix/lint.scm
@@ -917,9 +917,9 @@ descriptions maintained upstream."
(define exception-with-kind-and-args?
(const #f))))
-(define (check-derivation package)
+(define* (check-derivation package #:key store)
"Emit a warning if we fail to compile PACKAGE to a derivation."
- (define (try system)
+ (define (try store system)
(catch #t ;TODO: Remove 'catch' when Guile 2.x is no longer supported.
(lambda ()
(guard (c ((store-protocol-error? c)
@@ -938,25 +938,29 @@ descriptions maintained upstream."
(G_ "failed to create ~a derivation: ~a")
(list system
(condition-message c)))))
- (with-store store
- ;; Disable grafts since it can entail rebuilds.
- (parameterize ((%graft? #f))
- (package-derivation store package system #:graft? #f)
-
- ;; If there's a replacement, make sure we can compute its
- ;; derivation.
- (match (package-replacement package)
- (#f #t)
- (replacement
- (package-derivation store replacement system
- #:graft? #f)))))))
+ (parameterize ((%graft? #f))
+ (package-derivation store package system #:graft? #f)
+
+ ;; If there's a replacement, make sure we can compute its
+ ;; derivation.
+ (match (package-replacement package)
+ (#f #t)
+ (replacement
+ (package-derivation store replacement system
+ #:graft? #f))))))
(lambda args
(make-warning package
(G_ "failed to create ~a derivation: ~s")
(list system args)))))
- (filter lint-warning?
- (map try (package-supported-systems package))))
+ (define (check-with-store store)
+ (filter lint-warning?
+ (map (cut try store <>) (package-supported-systems package))))
+
+ ;; For backwards compatability, don't rely on store being set
+ (or (and=> store check-with-store)
+ (with-store store
+ (check-with-store store))))
(define (check-license package)
"Warn about type errors of the 'license' field of PACKAGE."
--
2.25.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 4/4] scripts: lint: Handle store connections for lint checkers.
2020-03-15 21:06 ` [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 2/4] lint: Mark the derivation checker as requiring a store connection Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 3/4] lint: Add a #:store argument to check-derivation Christopher Baines
@ 2020-03-15 21:06 ` Christopher Baines
2 siblings, 0 replies; 14+ messages in thread
From: Christopher Baines @ 2020-03-15 21:06 UTC (permalink / raw)
To: 38754
Rather than individual checkers opening up a connection to the store for each
package to check, if any checker requires a store connection, open a
connection and pass it to all checkers that would use it. This makes running
the derivation checker much faster for multiple packages.
* guix/scripts/lint.scm (run-checkers): Add a #:store argument, and pass the
store to checkers if they require a store connection.
(guix-lint): Establish a store connection if any checker requires one, and
pass it through to run-checkers.
---
guix/scripts/lint.scm | 38 ++++++++++++++++++++++++++++----------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/guix/scripts/lint.scm b/guix/scripts/lint.scm
index 8d08c484f5..97ffd57301 100644
--- a/guix/scripts/lint.scm
+++ b/guix/scripts/lint.scm
@@ -30,6 +30,7 @@
#:use-module (guix packages)
#:use-module (guix lint)
#:use-module (guix ui)
+ #:use-module (guix store)
#:use-module (guix scripts)
#:use-module (guix scripts build)
#:use-module (gnu packages)
@@ -53,7 +54,7 @@
(lint-warning-message lint-warning))))
warnings))
-(define (run-checkers package checkers)
+(define* (run-checkers package checkers #:key store)
"Run the given CHECKERS on PACKAGE."
(let ((tty? (isatty? (current-error-port))))
(for-each (lambda (checker)
@@ -63,7 +64,9 @@
(lint-checker-name checker))
(force-output (current-error-port)))
(emit-warnings
- ((lint-checker-check checker) package)))
+ (if (lint-checker-requires-store? checker)
+ ((lint-checker-check checker) package #:store store)
+ ((lint-checker-check checker) package))))
checkers)
(when tty?
(format (current-error-port) "\x1b[K")
@@ -167,12 +170,27 @@ run the checkers on all packages.\n"))
(_ #f))
(reverse opts)))
(checkers (or (assoc-ref opts 'checkers) %all-checkers)))
- (cond
- ((assoc-ref opts 'list?)
+
+ (when (assoc-ref opts 'list?)
(list-checkers-and-exit checkers))
- ((null? args)
- (fold-packages (lambda (p r) (run-checkers p checkers)) '()))
- (else
- (for-each (lambda (spec)
- (run-checkers (specification->package spec) checkers))
- args)))))
+
+ (let ((any-lint-checker-requires-store?
+ (any lint-checker-requires-store? checkers)))
+
+ (define (call-maybe-with-store proc)
+ (if any-lint-checker-requires-store?
+ (with-store store
+ (proc store))
+ (proc #f)))
+
+ (call-maybe-with-store
+ (lambda (store)
+ (cond
+ ((null? args)
+ (fold-packages (lambda (p r) (run-checkers p checkers
+ #:store store)) '()))
+ (else
+ (for-each (lambda (spec)
+ (run-checkers (specification->package spec) checkers
+ #:store store))
+ args))))))))
--
2.25.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2019-12-31 18:15 ` Ludovic Courtès
2020-03-15 21:06 ` [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record Christopher Baines
@ 2020-03-15 21:35 ` Christopher Baines
2020-03-24 10:20 ` Ludovic Courtès
1 sibling, 1 reply; 14+ messages in thread
From: Christopher Baines @ 2020-03-15 21:35 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 38754
[-- Attachment #1: Type: text/plain, Size: 2441 bytes --]
Ludovic Courtès <ludo@gnu.org> writes:
> Hi Chris!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Christopher Baines <mail@cbaines.net> skribis:
>>>
>>>> + (with-store store
>>>> + (parameterize
>>>> + ((%lint-checker-store-connection store))
>>>
>>> Actually it means that now ‘guix lint’ systematically connects to the
>>> daemon.
>>
>> I guess that's the effect, were you meaning this would make a better
>> message in the commit?
>
> I mean that it’s a visible change. Before, you could run all the
> linters but this one without having a daemon running; now you need a
> daemon up and running.
Ah, yeah, that's a good point.
>>> I wonder if we could arrange to open the connection lazily, and to
>>> somehow carry state across linter invocations. Perhaps
>>> ‘check-derivation’ should be monadic, with a field in <checker>
>>> indicating that. Sounds complicated though.
>>>
>>> Thoughts?
>>
>> I did wonder if the code could somehow transparently be made more
>> efficient. Quite often database clients manage a pool of connections,
>> and when you perform a database operation, a connection from the pool is
>> checked out, and then returned once you're finished. But as you say,
>> this could be complicated. I think parameters can be set with
>> connections, and I'm not quiet sure what the interface should be.
>>
>> I also did think about somehow passing the store connection in to the
>> lint checker more explicitly, but I'm not sure how to generalise that.
>
> There could be a <checker> field indicating either that (1) the
> procedure takes an optional store parameter, or that (2) the procedure
> is monadic in ‘%store-monad’.
>
> #2 seems more complicated to implement that #1 though.
>
> For #1, ‘guix lint’ could check whether:
>
> (any checker-require-store? checkers)
>
> is true, and if it is, it could open a connection and pass it on as
> needed.
>
> WDYT?
>
> If that seems good to you, I guess you can go ahead with it (let’s just
> not lose our hair on it!).
I've finally got around to looking at this again. I've sent some new
patches which add a field to the <lint-checker> record, and adjust the
running of lint checkers as well as the derivation checker to use a
single store connection.
Let me know what you think?
Thanks,
Chris
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2020-03-15 21:35 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
@ 2020-03-24 10:20 ` Ludovic Courtès
2020-03-24 19:50 ` bug#38754: " Christopher Baines
0 siblings, 1 reply; 14+ messages in thread
From: Ludovic Courtès @ 2020-03-24 10:20 UTC (permalink / raw)
To: Christopher Baines; +Cc: 38754
Hi,
Christopher Baines <mail@cbaines.net> skribis:
> I've finally got around to looking at this again. I've sent some new
> patches which add a field to the <lint-checker> record, and adjust the
> running of lint checkers as well as the derivation checker to use a
> single store connection.
>
> Let me know what you think?
LGTM, thank you!
Ludo’.
^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#38754: [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter.
2020-03-24 10:20 ` Ludovic Courtès
@ 2020-03-24 19:50 ` Christopher Baines
0 siblings, 0 replies; 14+ messages in thread
From: Christopher Baines @ 2020-03-24 19:50 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: 38754-done
[-- Attachment #1: Type: text/plain, Size: 503 bytes --]
Ludovic Courtès <ludo@gnu.org> writes:
> Hi,
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> I've finally got around to looking at this again. I've sent some new
>> patches which add a field to the <lint-checker> record, and adjust the
>> running of lint checkers as well as the derivation checker to use a
>> single store connection.
>>
>> Let me know what you think?
>
> LGTM, thank you!
Great, I've pushed this to master now as
57e12aad6dfc2d12567164144dd15161e66f32d5.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 962 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-03-24 19:51 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-26 17:33 [bug#38754] [PATCH 0/2] Speed up the derivation linter Christopher Baines
2019-12-26 18:01 ` [bug#38754] [PATCH 1/2] guix: lint: Add an optional parameter for a store connection Christopher Baines
2019-12-26 18:01 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
2019-12-30 22:12 ` Ludovic Courtès
2019-12-30 23:34 ` Christopher Baines
2019-12-31 18:15 ` Ludovic Courtès
2020-03-15 21:06 ` [bug#38754] [PATCH 1/4] lint: Add a requires-store? field to the checker record Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 2/4] lint: Mark the derivation checker as requiring a store connection Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 3/4] lint: Add a #:store argument to check-derivation Christopher Baines
2020-03-15 21:06 ` [bug#38754] [PATCH 4/4] scripts: lint: Handle store connections for lint checkers Christopher Baines
2020-03-15 21:35 ` [bug#38754] [PATCH 2/2] scripts: lint: Set the %link-checker-store-connection parameter Christopher Baines
2020-03-24 10:20 ` Ludovic Courtès
2020-03-24 19:50 ` bug#38754: " Christopher Baines
2019-12-30 22:01 ` [bug#38754] [PATCH 0/2] Speed up the derivation linter Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).