From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier 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 16:25:22 -0500 Message-ID: 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="35509"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Philip Kaludercic Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Mon Feb 14 22:26:15 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 1nJirO-00090E-QZ for ged-emacs-devel@m.gmane-mx.org; Mon, 14 Feb 2022 22:26:15 +0100 Original-Received: from localhost ([::1]:41612 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nJirN-0004zc-E3 for ged-emacs-devel@m.gmane-mx.org; Mon, 14 Feb 2022 16:26:13 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:56552) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nJiqm-0004Fh-FO for emacs-devel@gnu.org; Mon, 14 Feb 2022 16:25:36 -0500 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:45764) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nJiqj-0006te-Dc for emacs-devel@gnu.org; Mon, 14 Feb 2022 16:25:35 -0500 Original-Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 221344422F4; Mon, 14 Feb 2022 16:25:31 -0500 (EST) Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 008AB440161; Mon, 14 Feb 2022 16:25:24 -0500 (EST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1644873925; bh=ALcnF+5ri+oVGXmcRxcVm+bpIzztzs9bs+2T/y72A/g=; h=From:To:Cc:Subject:References:Date:In-Reply-To:From; b=FPWFZEwW+nNSSqvj4wcirsW5V3spcg8zCE7BEENin9rdhEpbmmiVAtSAPri/bJkFI huC9uwrLBQidcdk87wylN/BaSPJ0hhIi6G+wrDOIMM6BRnsfN/inQ8ICQSKPqnqAjS 100aTX8CNBEhOshPILDTO/m3vFDfw5x0G16QW5i+GbUYBmc7YH1VlJ3ZwO9VVLYUU8 d+1g0AroKrPGV+i2I0U7OFzM6pf2dDE/6B+rtjM90btIuhME268MWaRsZfkYFZ/smr 3SpY8ea/BhWzVVIwiQFD5sCFI3VucXBRwSfcjEFrHOOiMoKuO9CEyb9jO4dhjpAUqS TxWhOpYiyYdbw== Original-Received: from ceviche (modemcable085.122-83-70.mc.videotron.ca [70.83.122.85]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 7BDDB120920; Mon, 14 Feb 2022 16:25:24 -0500 (EST) In-Reply-To: <87bkz9tbcj.fsf@posteo.net> (Philip Kaludercic's message of "Mon, 14 Feb 2022 20:57:48 +0000") Received-SPF: pass client-ip=132.204.25.50; envelope-from=monnier@iro.umontreal.ca; helo=mailscanner.iro.umontreal.ca X-Spam_score_int: -42 X-Spam_score: -4.3 X-Spam_bar: ---- X-Spam_report: (-4.3 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-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:286301 Archived-At: >>> @@ -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" >>> +(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). >>> @@ -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. >>> + ;; 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. > 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. Stefan