Skip to content

Commit

Permalink
Fix several issues with snapshot VC package versions; adjust existing…
Browse files Browse the repository at this point in the history 
… tests so they catch them.
  • Loading branch information
@doublep
doublep committed Dec 8, 2024
1 parent 98349e8 commit 3671b55
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 40 deletions.
40 changes: 39 additions & 1 deletion eldev-vc.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ Since 1.2."
;; FIXME: Try to build stable versions from tags.
(when t
(setf stable nil))
(eldev-dump dir url default-directory)
(let ((package (eldev-package-descriptor dir)))
(unless stable
(eldev-call-process (eldev-git-executable) `("--no-pager" "log" "-1" "--no-color" "--format=%cI" "--no-patch")
Expand All @@ -242,6 +241,45 @@ Since 1.2."
,(string-to-number (format "%s%s" (match-string 4 date-string) (match-string 5 date-string))))))))))
package)))

(defun eldev--vc-install-as-package (vc-dependency)
;; Unlike with local dependencies, for VC-originated we generate and install Emacs
;; package here rather than when loading. The reason is that the source checkout is
;; controlled by Eldev and thus shouldn't outside.
(let ((tmp-package-dir (make-temp-file "eldev-vc-" t))
(package (cdr (assq (car vc-dependency) eldev--vc-dependency-packages))))
(eldev-verbose "Creating a package from `%s'" (eldev--vc-repository-name vc-dependency))
(let ((default-directory (eldev--vc-dependency-dir vc-dependency))
(display-stdout eldev-display-indirect-build-stdout)
;; Not using `--print-filename' here so that output can be better forwarded to
;; stdout if `eldev-display-indirect-build-stdout' asks for that.
(command-line `("package" "--output-dir" ,tmp-package-dir
"--force-version" ,(package-version-join (package-desc-version package))))
(setup (plist-get (cdr vc-dependency) :setup)))
(when setup
(setf command-line (append `("--setup" ,(prin1-to-string setup)) command-line)))
(eldev-call-process (eldev-shell-command) command-line
:forward-output (if display-stdout t 'stderr)
:destination (if display-stdout t '(t nil))
:trace-command-line (eldev-format-message "Full command line (in directory `%s')" default-directory)
:die-on-error (eldev-format-message "child Eldev process for VC dependency `%s'" (car vc-dependency)))
(unless display-stdout
(if (= (point-min) (point-max))
(eldev-verbose "Child Eldev process produced no output (other than maybe on stderr)")
(eldev-verbose "(Non-stderr) output of the child Eldev process:")
(eldev-verbose (buffer-string))))
(let ((generated (eldev-find-files '("./*.tar" "./*.el") t tmp-package-dir)))
(unless generated
(error "Child Eldev process succeeded, but apparently didn't generate a package"))
(when (cdr generated)
(error "Child Eldev process seems to have generated more than one (package) file"))
;; Not using `package-install-file' as it would ignore the forced version.
(let ((tmp-package (copy-sequence package)))
(setf (package-desc-kind tmp-package) (if (string-suffix-p ".tar" (car generated)) 'tar 'single))
(with-temp-buffer
(insert-file-contents (car generated))
(package-unpack tmp-package))))
(ignore-errors (delete-directory tmp-package-dir t)))))



;; Real work of `eldev init' command is moved here to make Eldev startup slightly faster:
Expand Down
33 changes: 2 additions & 31 deletions eldev.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
eldev-release-test-project eldev-release-maybe-fail)
("eldev-vc" eldev-vc-root-dir eldev-vc-executable eldev-vc-full-name eldev-with-vc eldev-with-vc-buffer eldev--vc-set-up-buffer eldev-vc-synchronize-dir
eldev-vc-detect eldev-vc-commit-id eldev-vc-branch-name
eldev--vc-fetch-repository)
eldev--vc-fetch-repository eldev--vc-install-as-package)
("eldev-doctor" eldev-defdoctest)))
(dolist (function (cdr autoloads))
(autoload function (car autoloads)))))
Expand Down Expand Up @@ -2759,36 +2759,7 @@ Since 0.2."
(eldev-named-step nil (eldev-format-message "installing dependency package `%s'" dependency-name)
(let ((inhibit-message t))
(if (string= (package-desc-archive dependency) eldev--internal-vc-pseudoarchive)
;; Unlike with local dependencies, for VC-originated we generate and install Emacs
;; package here rather than when loading. The reason is that the source checkout is
;; controlled by Eldev and thus shouldn't outside.
(let ((tmp-package-dir (make-temp-file "eldev-vc-" t)))
(eldev-verbose "Creating a package from `%s'" (eldev--vc-repository-name vc-dependency))
(let ((default-directory (eldev--vc-dependency-dir vc-dependency))
(display-stdout eldev-display-indirect-build-stdout)
;; Not using `--print-filename' here so that output can be better forwarded to
;; stdout if `eldev-display-indirect-build-stdout' asks for that.
(command-line `("package" "--output-dir" ,tmp-package-dir))
(setup (plist-get (cdr vc-dependency) :setup)))
(when setup
(setf command-line (append `("--setup" ,(prin1-to-string setup)) command-line)))
(eldev-call-process (eldev-shell-command) command-line
:forward-output (if display-stdout t 'stderr)
:destination (if display-stdout t '(t nil))
:trace-command-line (eldev-format-message "Full command line (in directory `%s')" default-directory)
:die-on-error (eldev-format-message "child Eldev process for VC dependency `%s'" dependency-name))
(unless display-stdout
(if (= (point-min) (point-max))
(eldev-verbose "Child Eldev process produced no output (other than maybe on stderr)")
(eldev-verbose "(Non-stderr) output of the child Eldev process:")
(eldev-verbose (buffer-string))))
(let ((generated (eldev-find-files '("./*.tar" "./*.el") t tmp-package-dir)))
(if generated
(if (cdr generated)
(error "Child Eldev process seems to have generated more than one (package) file")
(package-install-file (car generated)))
(error "Child Eldev process succeeded, but apparently didn't generate a package")))
(ignore-errors (delete-directory tmp-package-dir t))))
(eldev--vc-install-as-package vc-dependency)
(eldev--with-pa-access-workarounds (lambda ()
(eldev-using-global-package-archive-cache
(condition-case error
Expand Down
12 changes: 12 additions & 0 deletions test/common.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,18 @@ only with modification time."
(sleep-for 0.1))))


(defun eldev--test-unstable-version-rx (version &optional full-string)
"Create a regexp to match VERSION plus typical snapshot suffix.
I.e. plus “YYYYMMDD.HHMM” of the last commit. VERSION can be
either a string or a list."
(let ((regexp (if (stringp version)
(rx-to-string `(seq ,version ".2" (= 7 digit) "." (** 1 4 digit)))
(rx-to-string `(seq ,(substring (prin1-to-string version) 0 -1) " 2" (= 7 digit) " " (** 1 4 digit) ")")))))
(when full-string
(setf regexp (format "\\`%s\\'" regexp)))
regexp))


(defmacro eldev-ert-defargtest (name arguments values &rest body)
"Define a parameterized test with given NAME.
This is a poor-man's substitute for functionality missing from
Expand Down
30 changes: 22 additions & 8 deletions test/vc-dependencies.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@
(nil t)
(eldev--test-with-temp-copy "dependency-a" 'Git
(let ((dependency-a-dir eldev--test-project)
;; Unlike with local dependencies, exchanging archives must not affect installed
;; packages: they will remain untouched until you issue `upgrade' or `clean
;; ...'. So, the expected output is determined by the first run.
(expected-output (eldev--test-lines "\"Hello\"" (if from-pa-first "t" "nil") (if from-pa-first "(1 0)" "(1 0 99)")))
(eldev--test-project "vc-dep-project-a"))
(eldev--test-delete-cache)
(dolist (from-pa (if from-pa-first '(t nil) '(nil t)))
Expand All @@ -19,7 +15,14 @@
`(eldev-use-vc-dependency 'dependency-a :git ,dependency-a-dir))
"eval" `(dependency-a-hello) `(dependency-a-stable) `(package-desc-version (eldev-find-package-descriptor 'dependency-a)))
:description (if from-pa "Using package archive to resolve `dependency-a'" "Using Git repository to resolve `dependency-a'")
(should (string= stdout expected-output))
;; Unlike with local dependencies, exchanging archives must not affect installed
;; packages: they will remain untouched until you issue `upgrade' or `clean
;; ...'. So, the expected output is determined by the first run.
(should (string= (nth 0 (eldev--test-line-list stdout)) "\"Hello\""))
(should (string= (nth 1 (eldev--test-line-list stdout)) (if from-pa-first "t" "nil")))
(if from-pa-first
(should (string= (nth 2 (eldev--test-line-list stdout)) "(1 0)"))
(should (string-match-p (eldev--test-unstable-version-rx '(1 0 99) t) (nth 2 (eldev--test-line-list stdout)))))
(should (= exit-code 0)))))))


Expand All @@ -32,7 +35,9 @@
(eldev--test-run nil ("--setup" `(eldev-use-vc-dependency 'dependency-a :git ,dependency-a-dir)
"eval" `(dependency-a-hello) `(dependency-a-stable) `(package-desc-version (eldev-find-package-descriptor 'dependency-a)))
:description "Using original version of `dependency-a'"
(should (string= stdout (eldev--test-lines "\"Hello\"" "nil" "(1 0 99)")))
(should (equal (butlast (eldev--test-line-list stdout)) '("\"Hello\"" "nil")))
(should (string-match-p (eldev--test-unstable-version-rx '(1 0 99) t) (nth 2 (eldev--test-line-list stdout))))
(should (string-match-p (format "Installing.+dependency-a.+from.+%s" (regexp-quote dependency-a-dir)) stderr))
(should (= exit-code 0)))
(let ((default-directory dependency-a-dir))
(eldev-with-file-buffer "dependency-a.el"
Expand All @@ -43,15 +48,24 @@
"eval" `(dependency-a-hello) `(dependency-a-stable) `(package-desc-version (eldev-find-package-descriptor 'dependency-a)))
:description "After creating `dependency-a' 1.0.100, but before upgrading"
;; Upgrading VC dependencies must be explicit, just like for regular dependencies.
(should (string= stdout (eldev--test-lines "\"Hello\"" "nil" "(1 0 99)")))
(should (equal (butlast (eldev--test-line-list stdout)) '("\"Hello\"" "nil")))
(should (string-match-p (eldev--test-unstable-version-rx '(1 0 99) t) (nth 2 (eldev--test-line-list stdout))))
(should (= exit-code 0)))
(eldev--test-run nil (:eval `("--setup" ,`(eldev-use-vc-dependency 'dependency-a :git ,dependency-a-dir)
,@command))
:description "Upgrading"
(should (string-match-p (format "Upgrading.+dependency-a.+from.+%s" (regexp-quote dependency-a-dir)) stderr))
(should (= exit-code 0)))
(eldev--test-run nil ("--setup" `(eldev-use-vc-dependency 'dependency-a :git ,dependency-a-dir)
"eval" `(dependency-a-hello) `(dependency-a-stable) `(package-desc-version (eldev-find-package-descriptor 'dependency-a)))
:description "Using `dependency-a' 1.0.100"
(should (string= stdout (eldev--test-lines "\"Hello\"" "nil" "(1 0 100)")))
(should (equal (butlast (eldev--test-line-list stdout)) '("\"Hello\"" "nil")))
(should (string-match-p (eldev--test-unstable-version-rx '(1 0 100) t) (nth 2 (eldev--test-line-list stdout))))
(should (= exit-code 0)))
(eldev--test-run nil (:eval `("--setup" ,`(eldev-use-vc-dependency 'dependency-a :git ,dependency-a-dir)
,@command))
:description "Upgrading for the second time, must be a no-op"
(should (string= stdout (eldev--test-lines "All dependencies are up-to-date")))
(should (= exit-code 0))))))


Expand Down

0 comments on commit 3671b55

Please sign in to comment.