From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.devel Subject: Re: [elpa] master 872014e: Prevent accidental deletion of .git Date: Wed, 25 Nov 2015 22:55:09 -0500 Message-ID: References: <20151109013124.17711.29422@vcs.savannah.gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1448510135 24145 80.91.229.3 (26 Nov 2015 03:55:35 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Thu, 26 Nov 2015 03:55:35 +0000 (UTC) Cc: "=?windows-1252?Q?Fabi=E1n?= E. Gallina" , emacs-devel@gnu.org To: Thomas Fitzsimmons Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Thu Nov 26 04:55:19 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1a1neQ-00077I-1o for ged-emacs-devel@m.gmane.org; Thu, 26 Nov 2015 04:55:18 +0100 Original-Received: from localhost ([::1]:48953 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1neR-0000Ut-Ep for ged-emacs-devel@m.gmane.org; Wed, 25 Nov 2015 22:55:19 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:37595) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1neN-0000Uj-Ve for emacs-devel@gnu.org; Wed, 25 Nov 2015 22:55:16 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a1neK-0005Zm-Nq for emacs-devel@gnu.org; Wed, 25 Nov 2015 22:55:15 -0500 Original-Received: from ironport2-out.teksavvy.com ([206.248.154.181]:30689) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a1neK-0005Z5-JC; Wed, 25 Nov 2015 22:55:12 -0500 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A0A+FgA731xV/yr292hcgxCEAoVVwwsEAgKBPD0QAQEBAQEBAYEKQQWDXQEBAwFWIwULCw4mEhQYDSSINwjPIwEBCAEBAQEBHYs6hCBlB4QtBYwwhASkUCOBZiQcgXAggTSBRAEBAQ X-IPAS-Result: A0A+FgA731xV/yr292hcgxCEAoVVwwsEAgKBPD0QAQEBAQEBAYEKQQWDXQEBAwFWIwULCw4mEhQYDSSINwjPIwEBCAEBAQEBHYs6hCBlB4QtBYwwhASkUCOBZiQcgXAggTSBRAEBAQ X-IronPort-AV: E=Sophos;i="5.13,465,1427774400"; d="scan'208";a="179421162" Original-Received: from 104-247-246-42.cpe.teksavvy.com (HELO fmsmemgm.homelinux.net) ([104.247.246.42]) by ironport2-out.teksavvy.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 25 Nov 2015 22:55:09 -0500 Original-Received: by fmsmemgm.homelinux.net (Postfix, from userid 20848) id A2B7CAE065; Wed, 25 Nov 2015 22:55:09 -0500 (EST) In-Reply-To: (Thomas Fitzsimmons's message of "Wed, 25 Nov 2015 22:08:41 -0500") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 206.248.154.181 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 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.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:195273 Archived-At: >> We'd also want to try and fix the directory-deletion code accordingly, >> so it's less trigger happy. > I've been looking at how to do this, but I'm not sure exactly what > you're after. The following is from the perspective of "in place" > development. Yes, that's the important case. > There are lots of points where directory deletion happens in GNUmakefile > and archive-contents.el, during both "make externals" and "make > archive". Deletion in "make archive" is not a problem, AFAICT. > As for "make externals", I don't really like the concept of it, because > it mixes the build process with source configuration management. For > example, this directory deletion could remove in-progress work from my > source directory, under elpa/packages: Exactly. > ;; Check if `dir' is under version control. > ((not (zerop (call-process "git" nil nil nil > "ls-files" "--error-unmatch" dir))) > (message "Deleted untracked package %s" dir) > (delete-directory dir 'recursive t)) > > I would never expect that when invoking make. I would rather "git > status" tell me that I have untracked changes. Yes, this is the problematic case. This case is the one that intends to handle the situation where we removed a :core package from externals-list. Presumably the call-process check should make sure this directory is not under Git control at all, so no amount of "git status" will help. So here, instead of delete-directory we should use a new function which is a lot more careful, so it only deletes data which it has earlier created for a :core package. If the :core packages are handled via symlinks, then this can be done by checking that the directory only contains symlinks (or subdirs with the same property). Of course there are other ways, such as keeping track somewhere of the files we've installed as :core, and then check this info before we delete files. > I didn't closely follow the discussions while this was being designed. > Was there some reason to avoid git submodules? "git submodules" could make sense (and I did consider them) for the :external packages, but these aren't affected by this code (since the call-process should indicate that they are under Git). > It seems possible to replace "make externals" with submodules that > track "remote" externals/ branches of elpa.git itself, with git >= > 1.8.2. IIRC the issue with git submodules is that they have to refer to a particular revision rather than to a branch, so additionally to committing new code on the external branch, you'd then have to go and commit a change to master that updates the submodule reference to point to the new revision at the head of the external branch. Stefan