From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Kangas Newsgroups: gmane.emacs.bugs Subject: bug#7756: 24.0.50; enhancements to package.el Date: Sun, 15 Sep 2019 01:56:08 +0200 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="000000000000ffd60e05928c1df2" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="170675"; mail-complaints-to="usenet@blaine.gmane.org" Cc: 7756@debbugs.gnu.org, Richard Kim To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Sep 15 01:57:13 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1i9Huj-000iFs-7X for geb-bug-gnu-emacs@m.gmane.org; Sun, 15 Sep 2019 01:57:13 +0200 Original-Received: from localhost ([::1]:52720 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i9Huh-0002n4-Hf for geb-bug-gnu-emacs@m.gmane.org; Sat, 14 Sep 2019 19:57:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:34181) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1i9HuZ-0002m4-95 for bug-gnu-emacs@gnu.org; Sat, 14 Sep 2019 19:57:04 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1i9HuX-0007BT-U6 for bug-gnu-emacs@gnu.org; Sat, 14 Sep 2019 19:57:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:38407) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1i9HuX-0007BM-QZ for bug-gnu-emacs@gnu.org; Sat, 14 Sep 2019 19:57:01 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1i9HuX-0008UU-L9 for bug-gnu-emacs@gnu.org; Sat, 14 Sep 2019 19:57:01 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: Resent-From: Stefan Kangas Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 14 Sep 2019 23:57:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 7756 X-GNU-PR-Package: emacs Original-Received: via spool by 7756-submit@debbugs.gnu.org id=B7756.156850538932588 (code B ref 7756); Sat, 14 Sep 2019 23:57:01 +0000 Original-Received: (at 7756) by debbugs.gnu.org; 14 Sep 2019 23:56:29 +0000 Original-Received: from localhost ([127.0.0.1]:47225 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1i9Hu0-0008TY-Mx for submit@debbugs.gnu.org; Sat, 14 Sep 2019 19:56:29 -0400 Original-Received: from mail-pg1-f196.google.com ([209.85.215.196]:35943) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1i9Htx-0008TH-Ul for 7756@debbugs.gnu.org; Sat, 14 Sep 2019 19:56:26 -0400 Original-Received: by mail-pg1-f196.google.com with SMTP id m29so927147pgc.3 for <7756@debbugs.gnu.org>; Sat, 14 Sep 2019 16:56:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=TGumVwVs11QROVYhgNxCWG+fCb3fk7XlFDlyM93oyoo=; b=JfGBEKbPLFr/empatBamZcDt/epF4l2qbpCLOUm/Yss4fTcYQVsDDtP/Ujl20zzHiL qr1YgzVvUChBTWW4Jkni3m8nvUIK/TYX3z6BSP3La5l1Sp1lcthZWg7OVOea+dpV/gU1 BcsWB2GedIbxdadb4I60fph/xNyv1+0qRklY6FWivH4KsKzVfPvkf3SI6MdzVtLWDiEM Mt+CVjw8llytMWoTaWfPFJW64Ieadd8h8bc9kuQn7kpTSFmqhJsLchSOy6sovvMhN9wO hoOMrH5tDLnt8VUqxjvT049UzWtJlTxRqAZxcDXM0RzLxO+6BiQ5LYQNpJuLV3Df+J1L 8hWQ== X-Gm-Message-State: APjAAAVLxzqXlUlL+KUGk8JPZxE7zLLn3y0DUK5T+W+iy/4mJcF1zXnq AoKBR4gkq3MAKR24xVjzOdW7xG3mEiD0FVvU3z0= X-Google-Smtp-Source: APXvYqyMGTcm6svs2meKFL7pwW5V6spba0R4jfuVglndOE5lvBicgX3eWUxAnUzPc9eEL2ZSe7VbIabvSo9xN8BPdgw= X-Received: by 2002:aa7:8156:: with SMTP id d22mr37006132pfn.190.1568505379922; Sat, 14 Sep 2019 16:56:19 -0700 (PDT) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:166475 Archived-At: --000000000000ffd60e05928c1df2 Content-Type: text/plain; charset="UTF-8" Stefan Monnier writes: >> @@ -338,9 +338,14 @@ >> (pkg-file (expand-file-name >> (concat (package-strip-version package) "-pkg") >> pkg-dir))) >> - (when (and (file-directory-p pkg-dir) >> - (file-exists-p (concat pkg-file ".el"))) >> - (load pkg-file nil t)))) >> + ;; When one is creating a package and testing it out, it is easy >> + ;; to forget to add the -pkg.el file. When that happens, it would >> + ;; be useful to provide feedback to the user rather than silently >> + ;; failing. That is what (error ...) below is for. >> + (when (file-directory-p pkg-dir) >> + (if (file-exists-p (concat pkg-file ".el")) >> + (load pkg-file nil t) >> + (error "'%s' file is missing!"))))) > > I agree with the intention, but I wonder: why do we pass the `noerror' > parameter to `load' in the first place? This code seems to have changed substantially since this patch was proposed in 2010. Having looked at it now, I think naively raising an error here would risk breaking things. I therefore suggest to do nothing here for now. >> @@ -569,8 +574,16 @@ >> (defun package-unpack (name version) >> (let ((pkg-dir (expand-file-name (concat (symbol-name name) "-" version) >> package-user-dir))) >> + ;; Delete the package directory if it exists already. This may >> + ;; not be useful to the end users. However this is extremely >> + ;; important for package developers as they experiment with which >> + ;; files to include in a package. If a file is removed from one >> + ;; iteration to the next, then the presence of the unwanted elisp >> + ;; file could cause problems by polluting the generated autoload >> + ;; file. >> + (if (file-directory-p pkg-dir) >> + (delete-directory pkg-dir 'recursive)) > > Here, I also agree with the intention, but I'm a little uneasy blindly > removing a whole subdirectory without warning: I'd add a confirmation prompt. I have attached a patch which does the same but adds a confirmation prompt. I've also added a test case for this particular case. WDYT? Best regards, Stefan Kangas --000000000000ffd60e05928c1df2 Content-Type: text/x-patch; charset="US-ASCII"; name="0001-Prompt-to-delete-duplicate-package-directory-on-inst.patch" Content-Disposition: attachment; filename="0001-Prompt-to-delete-duplicate-package-directory-on-inst.patch" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_k0k7jkxo0 RnJvbSBiMTk2Mjg5OGRhYzRjODljNThkYjA2NmY1OThiNjU2YmE0MmY4ODA3IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBTdGVmYW4gS2FuZ2FzIDxzdGVmYW5rYW5nYXNAZ21haWwuY29t PgpEYXRlOiBTdW4sIDE1IFNlcCAyMDE5IDAxOjQ5OjM3ICswMjAwClN1YmplY3Q6IFtQQVRDSF0g UHJvbXB0IHRvIGRlbGV0ZSBkdXBsaWNhdGUgcGFja2FnZSBkaXJlY3Rvcnkgb24gaW5zdGFsbAoK KiBsaXNwL2VtYWNzLWxpc3AvcGFja2FnZS5lbCAocGFja2FnZS11bnBhY2spOiBQcm9tcHQgdG8g ZGVsZXRlCnBhY2thZ2UgZGlyZWN0b3J5IGlmIGl0IGFscmVhZHkgZXhpc3RzLiAgKEJ1ZyM3NzU2 KQoKKiB0ZXN0L2xpc3AvZW1hY3MtbGlzcC9wYWNrYWdlLXRlc3RzLmVsCihwYWNrYWdlLXRlc3Qt aW5zdGFsbC1tdWx0aS1maWxlL3BrZy1kaXItYWxyZWFkeS1leGlzdHMpOiBOZXcgdGVzdApjYXNl IGZvciBpbnN0YWxsYXRpb24gd2hlbiBwYWNrYWdlIGRpcmVjdG9yeSBhbHJlYWR5IGV4aXN0cy4K KHdpdGgtcGFja2FnZS10ZXN0KTogU3VwcG9ydCBhYm92ZSB0ZXN0IGNhc2UuCi0tLQogbGlzcC9l bWFjcy1saXNwL3BhY2thZ2UuZWwgICAgICAgICAgICB8ICA2ICsrKysrLQogdGVzdC9saXNwL2Vt YWNzLWxpc3AvcGFja2FnZS10ZXN0cy5lbCB8IDEyICsrKysrKysrKysrLQogMiBmaWxlcyBjaGFu Z2VkLCAxNiBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL2xpc3Av ZW1hY3MtbGlzcC9wYWNrYWdlLmVsIGIvbGlzcC9lbWFjcy1saXNwL3BhY2thZ2UuZWwKaW5kZXgg ZWYwYzUxNzFkZS4uMmFjMDdiYzA1NSAxMDA2NDQKLS0tIGEvbGlzcC9lbWFjcy1saXNwL3BhY2th Z2UuZWwKKysrIGIvbGlzcC9lbWFjcy1saXNwL3BhY2thZ2UuZWwKQEAgLTg4OCw3ICs4ODgsMTEg QEAgcGFja2FnZS11bnBhY2sKICAgICAgICAgICAgICAgIChpZiAoPiAobGVuZ3RoIGZpbGUtbGlz dCkgMSkgJ3RhciAnc2luZ2xlKSkpKQogICAgICAgKCd0YXIKICAgICAgICAobWFrZS1kaXJlY3Rv cnkgcGFja2FnZS11c2VyLWRpciB0KQotICAgICAgIDs7IEZJWE1FOiBzaG91bGQgd2UgZGVsZXRl IFBLRy1ESVIgaWYgaXQgZXhpc3RzPworICAgICAgICh3aGVuIChhbmQgKGZpbGUtZGlyZWN0b3J5 LXAgcGtnLWRpcikKKyAgICAgICAgICAgICAgICAgICh5ZXMtb3Itbm8tcAorICAgICAgICAgICAg ICAgICAgIChmb3JtYXQgIlBhY2thZ2UgZGlyZWN0b3J5IGFscmVhZHkgZXhpc3RzOiBgJXMnLiAg RGVsZXRlIGl0PyIKKyAgICAgICAgICAgICAgICAgICAgICAgICAgIChkaXJlY3RvcnktZmlsZS1u YW1lIHBrZy1kaXIpKSkpCisgICAgICAgICAoZGVsZXRlLWRpcmVjdG9yeSBwa2ctZGlyICdyZWN1 cnNpdmUpKQogICAgICAgIChsZXQqICgoZGVmYXVsdC1kaXJlY3RvcnkgKGZpbGUtbmFtZS1hcy1k aXJlY3RvcnkgcGFja2FnZS11c2VyLWRpcikpKQogICAgICAgICAgKHBhY2thZ2UtdW50YXItYnVm ZmVyIGRpcm5hbWUpKSkKICAgICAgICgnc2luZ2xlCmRpZmYgLS1naXQgYS90ZXN0L2xpc3AvZW1h Y3MtbGlzcC9wYWNrYWdlLXRlc3RzLmVsIGIvdGVzdC9saXNwL2VtYWNzLWxpc3AvcGFja2FnZS10 ZXN0cy5lbAppbmRleCBjNzU3YmNjZjY3Li5mZmUwNGU4ZmU3IDEwMDY0NAotLS0gYS90ZXN0L2xp c3AvZW1hY3MtbGlzcC9wYWNrYWdlLXRlc3RzLmVsCisrKyBiL3Rlc3QvbGlzcC9lbWFjcy1saXNw L3BhY2thZ2UtdGVzdHMuZWwKQEAgLTEwOSw3ICsxMDksOCBAQCB3aXRoLXBhY2thZ2UtdGVzdAog ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGluc3RhbGwKICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBsb2NhdGlvbgogICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHVwZGF0ZS1uZXdzCi0gICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdXBsb2FkLWJhc2UpCisgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgdXBsb2FkLWJhc2UKKyAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBjcmVhdGUtZGlyZWN0b3J5KQogICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAmcmVzdCBib2R5KQogICAiU2V0IHVwIHRlbXBvcmFy eSBsb2NhdGlvbnMgYW5kIHZhcmlhYmxlcyBmb3IgdGVzdGluZy4iCiAgIChkZWNsYXJlIChpbmRl bnQgMSkgKGRlYnVnICgoWyZyZXN0IGZvcm1dKSBib2R5KSkpCkBAIC0xMzgsNiArMTM5LDkgQEAg d2l0aC1wYWNrYWdlLXRlc3QKICAgICAgICAgICAgLChpZiBiYXNlZGlyIGAoY2QgLGJhc2VkaXIp KQogICAgICAgICAgICAodW5sZXNzIChmaWxlLWRpcmVjdG9yeS1wIHBhY2thZ2UtdXNlci1kaXIp CiAgICAgICAgICAgICAgKG1rZGlyIHBhY2thZ2UtdXNlci1kaXIpKQorICAgICAgICAgICAsKGlm IGNyZWF0ZS1kaXJlY3RvcnkKKyAgICAgICAgICAgICAgICBgKG1ha2UtZGlyZWN0b3J5IChleHBh bmQtZmlsZS1uYW1lICxjcmVhdGUtZGlyZWN0b3J5CisgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICBwYWNrYWdlLXRlc3QtdXNlci1kaXIpIHQpKQogICAg ICAgICAgICAoY2wtbGV0ZiAoKChzeW1ib2wtZnVuY3Rpb24gJ3llcy1vci1uby1wKSAobGFtYmRh ICgmcmVzdCByKSB0KSkKICAgICAgICAgICAgICAgICAgICAgICgoc3ltYm9sLWZ1bmN0aW9uICd5 LW9yLW4tcCkgICAgKGxhbWJkYSAoJnJlc3QgcikgdCkpKQogICAgICAgICAgICAgICxAKHdoZW4g aW5zdGFsbApAQCAtMzQ0LDYgKzM0OCwxMiBAQCBwYWNrYWdlLXRlc3QtaW5zdGFsbC1tdWx0aWZp bGUKICAgICAgICAgICAoZ290by1jaGFyIChwb2ludC1taW4pKQogICAgICAgICAgIChzaG91bGQg KHJlLXNlYXJjaC1mb3J3YXJkIHJlIG5pbCB0KSkpKSkpKQogCisoZXJ0LWRlZnRlc3QgcGFja2Fn ZS10ZXN0LWluc3RhbGwtbXVsdGktZmlsZS9wa2ctZGlyLWFscmVhZHktZXhpc3RzICgpCisgICJJ bnN0YWxsIG11bHRpLWZpbGUgcGFja2FnZSBvdmVyIGFscmVhZHkgZXhpc3RpbmcgZGlyZWN0b3J5 LiIKKyAgKHdpdGgtcGFja2FnZS10ZXN0ICg6YmFzZWRpciAicGFja2FnZS1yZXNvdXJjZXMiIDpp bnN0YWxsICcobXVsdGktZmlsZSkKKyAgICAgICAgICAgICAgICAgICAgICA6Y3JlYXRlLWRpcmVj dG9yeSAibXVsdGktZmlsZS0wLjIuMyIpCisgICAgKHNob3VsZCAocGFja2FnZS1pbnN0YWxsZWQt cCAnbXVsdGktZmlsZSkpKSkKKwogKGVydC1kZWZ0ZXN0IHBhY2thZ2UtdGVzdC11cGRhdGUtbGlz dGluZyAoKQogICAiRW5zdXJlIGluc3RhbGxlZCBwYWNrYWdlIHN0YXR1cyBpcyB1cGRhdGVkLiIK ICAgKHdpdGgtcGFja2FnZS10ZXN0ICgpCi0tIAoyLjIwLjEKCg== --000000000000ffd60e05928c1df2--