From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Philip Kaludercic Newsgroups: gmane.emacs.devel Subject: Re: feature/package+vc 04c4c578c7 3/4: Allow for packages to be installed directly from VCS Date: Mon, 14 Feb 2022 23:44:05 +0000 Message-ID: <87o839ypx6.fsf@posteo.net> References: <164484721900.31751.1453162457552427931@vcs2.savannah.gnu.org> <20220214140020.04438C00891@vcs2.savannah.gnu.org> <87bkz9tbcj.fsf@posteo.net> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="34529"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Feb 15 00:54:43 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nJlB5-0008nh-9o for ged-emacs-devel@m.gmane-mx.org; Tue, 15 Feb 2022 00:54:43 +0100 Original-Received: from localhost ([::1]:54986 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nJlB4-0007oO-7x for ged-emacs-devel@m.gmane-mx.org; Mon, 14 Feb 2022 18:54:42 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:59846) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nJl0t-0005h5-P6 for emacs-devel@gnu.org; Mon, 14 Feb 2022 18:44:11 -0500 Original-Received: from mout02.posteo.de ([185.67.36.66]:58519) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nJl0r-0004cX-0L for emacs-devel@gnu.org; Mon, 14 Feb 2022 18:44:11 -0500 Original-Received: from submission (posteo.de [185.67.36.169]) by mout02.posteo.de (Postfix) with ESMTPS id 76655240101 for ; Tue, 15 Feb 2022 00:44:07 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.net; s=2017; t=1644882247; bh=vvstPIc5Pj+Bsq7Mt01S6aEuJ0yTQBrShU6n540EyQM=; h=From:To:Cc:Subject:Autocrypt:Date:From; b=mXS6OlkbJ0SV5yDb+M6LLmGMATMm7+8+TewpQjXU9HCG7LtinSEcJ/Lj4Y+nCAhS9 7+SA8lmVTDFiYa/xplnhMlEX8WH9gPE2fHXf8zS5hglQsvkPToRTOkYmpzEqR1LejG c7yL5WpbPUFIdV9dIe/Ap7M2FglAnqMRsdfOJcQX98BDwSpEygDFfqnBA0OzPZIEqa ffLGx5EvPBCqScYrVAC3lX0OYR0rK6xM35gupGbJqhtTW2MOg4C8+SkMTA/JY9qy2g 9j0mvpAlMnUcVrMeCJnFzDVZJDzJGwhhkuPJQjErxMggA/RHgbrAWbGG1LfmtlX635 Y06SyXDbavHlg== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4JyLQt17Qvz6tlh; Tue, 15 Feb 2022 00:44:05 +0100 (CET) Autocrypt: addr=philipk@posteo.net; prefer-encrypt=nopreference; keydata= mDMEYHHqUhYJKwYBBAHaRw8BAQdAp3GdmYJ6tm5McweY6dEvIYIiry+Oz9rU4MH6NHWK0Ee0QlBo aWxpcCBLYWx1ZGVyY2ljIChnZW5lcmF0ZWQgYnkgYXV0b2NyeXB0LmVsKSA8cGhpbGlwa0Bwb3N0 ZW8ubmV0PoiQBBMWCAA4FiEEDM2H44ZoPt9Ms0eHtVrAHPRh1FwFAmBx6lICGwMFCwkIBwIGFQoJ CAsCBBYCAwECHgECF4AACgkQtVrAHPRh1FyTkgEAjlbGPxFchvMbxzAES3r8QLuZgCxeAXunM9gh io0ePtUBALVhh9G6wIoZhl0gUCbQpoN/UJHI08Gm1qDob5zDxnIHuDgEYHHqUhIKKwYBBAGXVQEF AQEHQNcRB+MUimTMqoxxMMUERpOR+Q4b1KgncDZkhrO2ql1tAwEIB4h4BBgWCAAgFiEEDM2H44Zo Pt9Ms0eHtVrAHPRh1FwFAmBx6lICGwwACgkQtVrAHPRh1Fw1JwD/Qo7kvtib8jy7puyWrSv0MeTS g8qIxgoRWJE/KKdkCLEA/jb9b9/g8nnX+UcwHf/4VfKsjExlnND3FrBviXUW6NcB In-Reply-To: (Stefan Monnier's message of "Mon, 14 Feb 2022 16:25:22 -0500") Received-SPF: pass client-ip=185.67.36.66; envelope-from=philipk@posteo.net; helo=mout02.posteo.de X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:286305 Archived-At: Stefan Monnier writes: >>>> @@ -560,7 +571,9 @@ This is, approximately, the inverse of `version-to-list'. >>>> This is the name of the package with its version appended." >>>> (format "%s-%s" >>>> (package-desc-name pkg-desc) >>>> - (package-version-join (package-desc-version pkg-desc)))) >>>> + (if (eq (package-desc-kind pkg-desc) 'source) >>>> + "devel" >>>> + (package-version-join (package-desc-version pkg-desc))))) >>> >>> Currently the way `package.el` handles the use of Git-installed packages >>> is that it allows a package's directory to be just `` instead of >>> `-`. >>> >>> So maybe we could do the same here. >> >> I guess the question here is if you want to make it explicit that >> e.g. when removing a package with package-delete, you will be removing a >> local checkout that might have modifications. If not, it would probably >> make sense to add warnings where necessary. > > I don't understand why using `-devel` instead of `` makes it > easier to know that "you will be removing a local checkout that might > have modifications" The -devel doesn't necessarily indicate that directly, I just meant it was worth somehow explicitly indicating that this is a VCS-package. >>>> +(declare-function vc-working-revision "vc" (file &optional backend)) >>>> +(defun package-devel-commit (pkg) >>>> + "Extract the commit of a development package PKG." >>>> + (cl-assert (eq (package-desc-kind pkg) 'source)) >>>> + (require 'vc) >>>> + (cl-loop with dir = (package-desc-dir pkg) >>>> + for file in (directory-files dir t "\\.el\\'" t) >>>> + when (vc-working-revision file) return it >>>> + finally return "unknown")) >>> >>> Any chance we could use `vc-working-revision` on the package's root >>> directory to avoid this loop? >> >> If you pass the backend directly to vc-working-revision, and set the >> file to the expanded file name of the current directory, it seems to >> work (at least for git and SVN). Would it make sense to add that as a >> first step, then fall back to the loop? > > Since you say it works, yes. We could even drop the loop altogether > (and either not accept working with those VC backends where it doesn't > work, or improve those backends which we do want to work). As I presume that in most cases Git is used, and this appears to be working with vc-git, this should be ok. >>>> @@ -681,6 +704,14 @@ return it." >>>> (read (current-buffer))) >>>> (error "Can't find define-package in %s" pkg-file)))) >>>> (setf (package-desc-dir pkg-desc) pkg-dir) >>>> + (when (file-exists-p (expand-file-name >>>> + (symbol-name (package-desc-name pkg-desc)) >>>> + package-devel-dir)) >>>> + ;; XXX: This check seems dirty, there should be a better >>>> + ;; way to deduce if a package is in the devel directory. >>>> + (setf (package-desc-kind pkg-desc) 'source) >>>> + (push (cons :commit (package-devel-commit pkg-desc)) >>>> + (package-desc-extras pkg-desc))) >>>> (if (file-exists-p signed-file) >>>> (setf (package-desc-signed pkg-desc) t)) >>>> pkg-desc))))) >>> >>> Hmm... why do we need to know if a package is in the devel directory? >> >> Currently the :kind is not stored, so we have to infer it somehow. I am >> certain this could also be solve some other way, but I would also like >> to allow for simply placing a directory into elpa/devel to be handled as >> a package, as before with my site-lisp.el proposal. > > IIUC for those VCS-installed packages we generate the -pkg.el file > ourselves, so it should be fairly easy to make sure that file provides > the needed info so we don't need to guess. Yes, but again, if I just clone a project into elpa/devel, this won't work. It could be argued that this shouldn't be supported because it is a different issue, and the elpa/... directories maintained by package.el, but in that case I would like some other way to have package.el manage non-versioned local source. >>>> + ;; In case the package was installed directly from source, the >>>> + ;; dependency list wasn't know beforehand, and they might have >>>> + ;; to be installed explicitly. >>>> + (let (deps) >>>> + (dolist (file (directory-files pkg-dir t "\\.el\\'" t)) >>>> + (with-temp-buffer >>>> + (insert-file-contents file) >>>> + (when-let* ((require-lines (lm-header-multiline "package-requires"))) >>>> + (thread-last >>>> + (mapconcat #'identity require-lines " ") >>>> + package-read-from-string >>>> + package--prepare-dependencies >>>> + (nconc deps) >>>> + (setq deps))))) >>> >>> Hmm... I expected here we'd open the `.el` file and call >>> `package-buffer-info`. >> >> That was my first thought too, but are we always certain that .el >> is the main file? > > For the rare cases where this is not the case, the `:main-file` property > of the package spec should tell us which file it is. I'd rather not try > and guess. Ok, I didn't know that :main-file was propagated into the package specification. In that case there should be no issue. >> That would also be possible, I believe I initially assumed that :vc >> would contain the contents of the header, and forgot to revise that >> assumption later on. > > I think making it hold the package spec (as given in `elpa-packages`) > would be a natural starting point. Ok. -- Philip Kaludercic