* Re: [elpa] master 872014e: Prevent accidental deletion of .git [not found] ` <E1ZvbIq-0004cL-VV@vcs.savannah.gnu.org> @ 2015-11-09 1:58 ` Stefan Monnier 2015-11-09 4:49 ` Thomas Fitzsimmons 0 siblings, 1 reply; 17+ messages in thread From: Stefan Monnier @ 2015-11-09 1:58 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: Fabián E. Gallina, emacs-devel > +# Use && after the cd commands, not ;, to ensure the build fails > +# immediately if the directory $(ARCHIVE_TMP)/packages does not exist. > +# For process-archive this is crucial; otherwise batch-make-archive in > +# archive-contents.el will interpret directories in the current > +# directory as unreleased packages, and recursively delete them, > +# including .git. Prior to using &&, running "make process-archive" > +# could silently delete all local git history! Actually, I think the problem is in the code which does the deletion: it should only do the delete files it positively knows should be deleted, rather than deleting any files which seem to be out of place. Once we change the handling of :core in elpa/admin/archive-contents.el so that it sets up symlinks rather than performing copies, we should at least be able to make the deletion less dangerous, e.g. by only deleting symlinks and empty directories (so even if the deletion was erroneous, no actual file contents was destroyed along the way). Fabián, any news on this? Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 1:58 ` [elpa] master 872014e: Prevent accidental deletion of .git Stefan Monnier @ 2015-11-09 4:49 ` Thomas Fitzsimmons 2015-11-09 5:03 ` Stefan Monnier 0 siblings, 1 reply; 17+ messages in thread From: Thomas Fitzsimmons @ 2015-11-09 4:49 UTC (permalink / raw) To: Stefan Monnier; +Cc: Fabián E. Gallina, emacs-devel [-- Attachment #1: Type: text/plain, Size: 1840 bytes --] Stefan Monnier <monnier@iro.umontreal.ca> writes: >> +# Use && after the cd commands, not ;, to ensure the build fails >> +# immediately if the directory $(ARCHIVE_TMP)/packages does not exist. >> +# For process-archive this is crucial; otherwise batch-make-archive in >> +# archive-contents.el will interpret directories in the current >> +# directory as unreleased packages, and recursively delete them, >> +# including .git. Prior to using &&, running "make process-archive" >> +# could silently delete all local git history! > > Actually, I think the problem is in the code which does the deletion: it > should only do the delete files it positively knows should be deleted, > rather than deleting any files which seem to be out of place. Agreed that the deletion code is questionable, though I guess it's operating under the assumption that it's within a throwaway directory and is erring on the side of not publishing something not intended for publication. I think using && after the cd is just good practice, and I wanted to push at least a partial fix for this right away since it bit me today. > Once we change the handling of :core in elpa/admin/archive-contents.el > so that it sets up symlinks rather than performing copies, we should at > least be able to make the deletion less dangerous, e.g. by only deleting > symlinks and empty directories (so even if the deletion was erroneous, > no actual file contents was destroyed along the way). > > Fabián, any news on this? Arthur pointed out in another thread that this code is at least partially there already (I haven't heard from Fabián about its status). The attached patch seems to work with a locally-generated archive, but it introduces the first external to make use of :core. Should I push it and see what happens? Thomas [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Add-ntlm-to-externals-list.patch --] [-- Type: text/x-patch, Size: 878 bytes --] From 4058dd04d88120b914a65f756356ac1aad42f224 Mon Sep 17 00:00:00 2001 From: Thomas Fitzsimmons <fitzsim@fitzsim.org> Date: Sun, 8 Nov 2015 23:33:49 -0500 Subject: [PATCH] Add ntlm to externals-list * externals-list ("ntlm"): New core entry. --- externals-list | 1 + 1 file changed, 1 insertion(+) diff --git a/externals-list b/externals-list index ea0a69c..dd9ea61 100644 --- a/externals-list +++ b/externals-list @@ -59,6 +59,7 @@ ("math-symbol-lists" :subtree "https://github.com/vspinu/math-symbol-lists.git") ("nameless" :subtree "https://github.com/Malabarba/Nameless") ("names" :subtree "http://github.com/Bruce-Connor/names") + ("ntlm" :core "lisp/net/ntlm.el") ("omn-mode" :external nil) ("pabbrev" :external "https://github.com/phillord/pabbrev.git") ("pinentry" :subtree "https://github.com/ueno/pinentry-el.git") -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 4:49 ` Thomas Fitzsimmons @ 2015-11-09 5:03 ` Stefan Monnier 2015-11-09 16:07 ` Eli Zaretskii 2015-11-18 14:39 ` Thomas Fitzsimmons 0 siblings, 2 replies; 17+ messages in thread From: Stefan Monnier @ 2015-11-09 5:03 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: Fabián E. Gallina, emacs-devel >> Once we change the handling of :core in elpa/admin/archive-contents.el >> so that it sets up symlinks rather than performing copies, we should at >> least be able to make the deletion less dangerous, e.g. by only deleting >> symlinks and empty directories (so even if the deletion was erroneous, >> no actual file contents was destroyed along the way). >> Fabián, any news on this? > Arthur pointed out in another thread that this code is at least > partially there already (I haven't heard from Fabián about its status). Right, Fabian installed its code already, but I found out at that point that the way it works (i.e copying files from emacs.git) is problematic in various ways: it should instead setup symlinks. > The attached patch seems to work Yes, it kinda works but it suffers from various warts (e.g. "make externals; make" will always recompile all the :core packages). > with a locally-generated archive, but it introduces the first external > to make use of :core. Should I push it and see what happens? No, we should change to code to use symlinks instead of file-copies first (and at the same time change it as I suggested, i.e. to only delete symlinks and empty directories). Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 5:03 ` Stefan Monnier @ 2015-11-09 16:07 ` Eli Zaretskii 2015-11-09 16:38 ` Artur Malabarba 2015-11-18 14:39 ` Thomas Fitzsimmons 1 sibling, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2015-11-09 16:07 UTC (permalink / raw) To: Stefan Monnier; +Cc: fgallina, fitzsim, emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Date: Mon, 09 Nov 2015 00:03:34 -0500 > Cc: "Fabián E. Gallina" <fgallina@gnu.org>, > emacs-devel@gnu.org > > Right, Fabian installed its code already, but I found out at that point > that the way it works (i.e copying files from emacs.git) is problematic > in various ways: it should instead setup symlinks. Symlinks will cause problems on MS-Windows. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 16:07 ` Eli Zaretskii @ 2015-11-09 16:38 ` Artur Malabarba 2015-11-09 16:45 ` Eli Zaretskii 0 siblings, 1 reply; 17+ messages in thread From: Artur Malabarba @ 2015-11-09 16:38 UTC (permalink / raw) To: Eli Zaretskii; +Cc: fgallina, fitzsim, Stefan Monnier, emacs-devel Eli Zaretskii <eliz@gnu.org> writes: >> Right, Fabian installed its code already, but I found out at that point >> that the way it works (i.e copying files from emacs.git) is problematic >> in various ways: it should instead setup symlinks. > > Symlinks will cause problems on MS-Windows. IIUC, this would only be a problem if you want to build a mirror of the Gelpa repository on a Windows machine. It wouldn't affect Windows users trying to install packages from elpa.gnu.org. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 16:38 ` Artur Malabarba @ 2015-11-09 16:45 ` Eli Zaretskii 2015-11-09 20:20 ` Stefan Monnier 0 siblings, 1 reply; 17+ messages in thread From: Eli Zaretskii @ 2015-11-09 16:45 UTC (permalink / raw) To: Artur Malabarba; +Cc: fgallina, fitzsim, monnier, emacs-devel > From: Artur Malabarba <bruce.connor.am@gmail.com> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, fgallina@gnu.org, fitzsim@fitzsim.org, emacs-devel@gnu.org > Date: Mon, 09 Nov 2015 16:38:45 +0000 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> Right, Fabian installed its code already, but I found out at that point > >> that the way it works (i.e copying files from emacs.git) is problematic > >> in various ways: it should instead setup symlinks. > > > > Symlinks will cause problems on MS-Windows. > > IIUC, this would only be a problem if you want to build a mirror of the > Gelpa repository on a Windows machine. It wouldn't affect Windows users > trying to install packages from elpa.gnu.org. Maybe I've misunderstood, but I thought it will affect everyone who has a clone of Emacs and a clone of ELPA. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 16:45 ` Eli Zaretskii @ 2015-11-09 20:20 ` Stefan Monnier 0 siblings, 0 replies; 17+ messages in thread From: Stefan Monnier @ 2015-11-09 20:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: fgallina, fitzsim, Artur Malabarba, emacs-devel > Maybe I've misunderstood, but I thought it will affect everyone who > has a clone of Emacs and a clone of ELPA. That will affect Windows users who make to use "make externals" in elpa.git, yes. We'll have to fall back to copying in that case. Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-09 5:03 ` Stefan Monnier 2015-11-09 16:07 ` Eli Zaretskii @ 2015-11-18 14:39 ` Thomas Fitzsimmons 2015-11-18 15:10 ` Stefan Monnier 1 sibling, 1 reply; 17+ messages in thread From: Thomas Fitzsimmons @ 2015-11-18 14:39 UTC (permalink / raw) To: Stefan Monnier; +Cc: Fabián E. Gallina, emacs-devel [-- Attachment #1: Type: text/plain, Size: 398 bytes --] Stefan Monnier <monnier@iro.umontreal.ca> writes: [...] > No, we should change to code to use symlinks instead of > file-copies first (and at the same time change it as I suggested, > i.e. to only delete symlinks and empty directories). How about the attached? I've only tested it on a single-file package (ntlm.el). I could extend it to directories if the approach is what you want. Thomas [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Use-symbolic-links-for-core-packages.patch --] [-- Type: text/x-patch, Size: 3099 bytes --] From 73978a3d5c49c59e02b42000de6c8d9337e6c83b Mon Sep 17 00:00:00 2001 From: Thomas Fitzsimmons <fitzsim@fitzsim.org> Date: Sun, 15 Nov 2015 21:25:04 -0500 Subject: [PATCH] Use symbolic links for core packages * admin/archive-contents.el (archive--process-simple-package): Copy file if it is a symlink to prevent deletion of target source file. (archive--core-package-copy-file): Create symbolic links for core files. (archive--core-package-sync): Change message to indicate linking. --- admin/archive-contents.el | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/admin/archive-contents.el b/admin/archive-contents.el index 74e473e..4f3aee9 100755 --- a/admin/archive-contents.el +++ b/admin/archive-contents.el @@ -207,8 +207,11 @@ (defun archive--process-simple-package (dir pkg vers desc req extras) "Deploy the contents of DIR into the archive as a simple package. Rename DIR/PKG.el to PKG-VERS.el, delete DIR, and return the descriptor." ;; Write DIR/foo.el to foo-VERS.el and delete DIR - (rename-file (expand-file-name (concat pkg ".el") dir) - (concat pkg "-" vers ".el")) + (if (file-symlink-p (expand-file-name (concat pkg ".el") dir)) + (copy-file (expand-file-name (concat pkg ".el") dir) + (concat pkg "-" vers ".el")) + (rename-file (expand-file-name (concat pkg ".el") dir) + (concat pkg "-" vers ".el"))) ;; Add the content of the ChangeLog. (let ((cl (expand-file-name "ChangeLog" dir))) (with-current-buffer (find-file-noselect (concat pkg "-" vers ".el")) @@ -647,7 +650,7 @@ (defun archive--core-package-empty-dest-p (dest) (defun archive--core-package-copy-file (source dest emacs-repo-root package-root exclude-regexp) - "Copy file from SOURCE to DEST ensuring subdirectories." + "Link file from SOURCE to DEST ensuring subdirectories." (unless (string-match-p exclude-regexp source) (let* ((absolute-package-file-name (expand-file-name dest package-root)) @@ -656,7 +659,10 @@ (defun archive--core-package-copy-file (directory (file-name-directory absolute-package-file-name))) (unless (file-directory-p directory) (make-directory directory t)) - (copy-file absolute-core-file-name absolute-package-file-name)) + (if (memq system-type '(windows-nt ms-dos)) + (copy-file absolute-core-file-name absolute-package-file-name) + (make-symbolic-link absolute-core-file-name + absolute-package-file-name t))) (message " %s -> %s" source (if (archive--core-package-empty-dest-p dest) (file-name-nondirectory source) dest)))) @@ -711,7 +717,7 @@ (defun archive--core-package-sync (definition) ;; Files may be just a string, normalize. (list file-patterns) file-patterns)))) - (message "Copying files for package: %s" name) + (message "Linking files for package: %s" name) (when (file-directory-p package-root) (delete-directory package-root t)) (make-directory package-root t) -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-18 14:39 ` Thomas Fitzsimmons @ 2015-11-18 15:10 ` Stefan Monnier 2015-11-18 16:44 ` Stephen Leake ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Stefan Monnier @ 2015-11-18 15:10 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: Fabián E. Gallina, emacs-devel > How about the attached? Looks like a good start, thank you. > @@ -207,8 +207,11 @@ (defun archive--process-simple-package (dir pkg vers desc req extras) > "Deploy the contents of DIR into the archive as a simple package. > Rename DIR/PKG.el to PKG-VERS.el, delete DIR, and return the descriptor." > ;; Write DIR/foo.el to foo-VERS.el and delete DIR > - (rename-file (expand-file-name (concat pkg ".el") dir) > - (concat pkg "-" vers ".el")) > + (if (file-symlink-p (expand-file-name (concat pkg ".el") dir)) > + (copy-file (expand-file-name (concat pkg ".el") dir) > + (concat pkg "-" vers ".el")) > + (rename-file (expand-file-name (concat pkg ".el") dir) > + (concat pkg "-" vers ".el"))) I'd use (let ((src (expand-file-name (concat pkg ".el") dir))) (funcall (if (file-symlink-p src) #'copy-file #'rename-file) src (concat pkg "-" vers ".el"))) to avoid redundancy. > (defun archive--core-package-copy-file > (source dest emacs-repo-root package-root exclude-regexp) > - "Copy file from SOURCE to DEST ensuring subdirectories." > + "Link file from SOURCE to DEST ensuring subdirectories." The function name would need to be correspondingly adjusted ;-) > - (copy-file absolute-core-file-name absolute-package-file-name)) > + (if (memq system-type '(windows-nt ms-dos)) > + (copy-file absolute-core-file-name absolute-package-file-name) > + (make-symbolic-link absolute-core-file-name + > absolute-package-file-name t))) I don't have a Windows system to test it, but if possible I'd rather have something like (if (fboundp 'make-symbolic-link) ...) or (condition-case nil (make-symbolic-link absolute-core-file-name absolute-package-file-name t) (<the-error-signalled-by-make-symbolic-link> (copy-file absolute-core-file-name absolute-package-file-name))) We'd also want to try and fix the directory-deletion code accordingly, so it's less trigger happy. Stefan "who also got caught by this directory deletion code" ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-18 15:10 ` Stefan Monnier @ 2015-11-18 16:44 ` Stephen Leake 2015-11-18 17:35 ` Eli Zaretskii 2015-11-26 3:08 ` Thomas Fitzsimmons 2 siblings, 0 replies; 17+ messages in thread From: Stephen Leake @ 2015-11-18 16:44 UTC (permalink / raw) To: Stefan Monnier; +Cc: Fabián E. Gallina, Thomas Fitzsimmons, emacs-devel Stefan Monnier <monnier@IRO.UMontreal.CA> writes: > I don't have a Windows system to test it, but if possible I'd rather > have something like > > (if (fboundp 'make-symbolic-link) ...) make-symbolic-link is bound on Windows. > or > > (condition-case nil > (make-symbolic-link absolute-core-file-name > absolute-package-file-name t) > (<the-error-signalled-by-make-symbolic-link> > (copy-file absolute-core-file-name absolute-package-file-name))) The error is 'file-error -- -- Stephe ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-18 15:10 ` Stefan Monnier 2015-11-18 16:44 ` Stephen Leake @ 2015-11-18 17:35 ` Eli Zaretskii 2015-11-26 3:08 ` Thomas Fitzsimmons 2 siblings, 0 replies; 17+ messages in thread From: Eli Zaretskii @ 2015-11-18 17:35 UTC (permalink / raw) To: Stefan Monnier; +Cc: fgallina, fitzsim, emacs-devel > From: Stefan Monnier <monnier@IRO.UMontreal.CA> > Date: Wed, 18 Nov 2015 10:10:26 -0500 > Cc: "Fabián E. Gallina" <fgallina@gnu.org>, > emacs-devel@gnu.org > > > - (copy-file absolute-core-file-name absolute-package-file-name)) > > + (if (memq system-type '(windows-nt ms-dos)) > > + (copy-file absolute-core-file-name absolute-package-file-name) > > + (make-symbolic-link absolute-core-file-name + > > absolute-package-file-name t))) > > I don't have a Windows system to test it, but if possible I'd rather > have something like > > (if (fboundp 'make-symbolic-link) ...) That'd be okay for the MS-DOS case, but not for Windows: we do have this function implemented there. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-18 15:10 ` Stefan Monnier 2015-11-18 16:44 ` Stephen Leake 2015-11-18 17:35 ` Eli Zaretskii @ 2015-11-26 3:08 ` Thomas Fitzsimmons 2015-11-26 3:55 ` Stefan Monnier 2 siblings, 1 reply; 17+ messages in thread From: Thomas Fitzsimmons @ 2015-11-26 3:08 UTC (permalink / raw) To: Stefan Monnier; +Cc: Fabián E. Gallina, emacs-devel Hi Stefan, Stefan Monnier <monnier@IRO.UMontreal.CA> writes: [...] > 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. For the deployment case, deletions etc. make perfect sense since those scripts are operating on committed code. There are lots of points where directory deletion happens in GNUmakefile and archive-contents.el, during both "make externals" and "make archive". For "make archive", directory deletions all happen under "archive-tmp" -- this is now enforced by my change to use cd ... && ... in the make recipe. The specific directory deletion that bit me is in batch-make-achive: (progn ;; Negative version: don't publish this package yet! (message "Package %s not released yet!" dir) (delete-directory dir 'recursive)) To avoid `delete-directory' calls entirely, we could create archive-tmp additively by only copying over releasable directories. But then the first step will still be "rm -rf archive-tmp" in the Makefile. Do you think it's worth the effort to rewrite this to do selective copying, rather than just ensuring directory deletions only happen in archive-tmp? 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: ;; 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. I didn't closely follow the discussions while this was being designed. Was there some reason to avoid git submodules? It seems possible to replace "make externals" with submodules that track "remote" externals/ branches of elpa.git itself, with git >= 1.8.2. Then pure git operations could be used to e.g. ensure a clean working tree, instead of directory deletion Elisp. Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-26 3:08 ` Thomas Fitzsimmons @ 2015-11-26 3:55 ` Stefan Monnier 2015-11-26 14:34 ` Thomas Fitzsimmons 0 siblings, 1 reply; 17+ messages in thread From: Stefan Monnier @ 2015-11-26 3:55 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: Fabián E. Gallina, emacs-devel >> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-26 3:55 ` Stefan Monnier @ 2015-11-26 14:34 ` Thomas Fitzsimmons 2015-11-26 15:42 ` Stefan Monnier 0 siblings, 1 reply; 17+ messages in thread From: Thomas Fitzsimmons @ 2015-11-26 14:34 UTC (permalink / raw) To: Stefan Monnier; +Cc: Fabián E. Gallina, emacs-devel Stefan Monnier <monnier@iro.umontreal.ca> writes: >>> 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. BTW, I found myself being more focused on the "in place archive" use case. That's how I ended up invoking the process-archive target directly which indirectly deleted .git (this is the one my patch now prevents). I think the default "make" target should also generate the archive directory so that one can always test "what will be published" with: rm -rf test-home && mkdir test-home && HOME=`pwd`/test-home $TARGET -Q \ --eval \ "(setq package-archives '((\"local\" . \".../elpa/archive/packages\")))" Then one can test dependency resolution, byte-compilation and compatibility on the "target Emacs" -- I test Emacs 24.1 through 24.5 and master. Testing just against the default emacs on PATH (what "make" does right now) is OK for development, but GNU ELPA should encourage backward and forward compatibility. >> 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. OK, I'll work on this. >> 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. That was true at one point but it looks like git 1.8.2 introduced the notion of "remote-tracking branch submodules". Maybe I'll experiment with how that could work for elpa.git. Thomas ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-26 14:34 ` Thomas Fitzsimmons @ 2015-11-26 15:42 ` Stefan Monnier 2015-11-27 6:58 ` Thomas Fitzsimmons 0 siblings, 1 reply; 17+ messages in thread From: Stefan Monnier @ 2015-11-26 15:42 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: Fabián E. Gallina, emacs-devel > BTW, I found myself being more focused on the "in place archive" use > case. I have no idea what "in-place archive" means here. AFAIK none of the code lets you generate an archive without making copies of the files (i.e. it's not "in-place"). > That's how I ended up invoking the process-archive target > directly which indirectly deleted .git (this is the one my patch now > prevents). [...] > Then one can test dependency resolution, byte-compilation and > compatibility on the "target Emacs" -- I test Emacs 24.1 through 24.5 > and master. Testing just against the default emacs on PATH (what "make" > does right now) is OK for development, but GNU ELPA should encourage > backward and forward compatibility. AFAIK "process-archive" won't byte-compile anything, so it's not very useful for that. It just shuffles things around so they're in the place and form expected for package-install. The purpose of the "in-place installation" method is so that those packages behave kind of like Emacs's bundled packages (except that you need to add the "packages" dir to your package-directory-list, so they won't appear in "emacs -Q" and after "cd .../emacs; make" you also have to do "cd .../elpa; make"). Just like Emacs's bundled packages, you don't need to "package-install" them, or even to choose which ones you want. Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-26 15:42 ` Stefan Monnier @ 2015-11-27 6:58 ` Thomas Fitzsimmons 2015-11-27 15:27 ` Stefan Monnier 0 siblings, 1 reply; 17+ messages in thread From: Thomas Fitzsimmons @ 2015-11-27 6:58 UTC (permalink / raw) To: Stefan Monnier; +Cc: Fabián E. Gallina, emacs-devel [-- Attachment #1: Type: text/plain, Size: 2854 bytes --] Stefan Monnier <monnier@IRO.UMontreal.CA> writes: >> BTW, I found myself being more focused on the "in place archive" use >> case. > > I have no idea what "in-place archive" means here. AFAIK none of the > code lets you generate an archive without making copies of the files > (i.e. it's not "in-place"). OK, wasn't sure what to call it. I meant invoking "make archive", which creates the archive in the elpa working directory, including local changes. Basically, I was doing: make externals make make archive >> That's how I ended up invoking the process-archive target >> directly which indirectly deleted .git (this is the one my patch now >> prevents). > [...] >> Then one can test dependency resolution, byte-compilation and >> compatibility on the "target Emacs" -- I test Emacs 24.1 through 24.5 >> and master. Testing just against the default emacs on PATH (what "make" >> does right now) is OK for development, but GNU ELPA should encourage >> backward and forward compatibility. > > AFAIK "process-archive" won't byte-compile anything, so it's not very > useful for that. It just shuffles things around so they're in the place > and form expected for package-install. Agreed, didn't mean to confuse things with the reference to "process-archive". I only called it while I was experimenting with how the build process works. I don't call it directly normally. But for byte-compilation testing, I do a package-install in each target emacs, and check the byte-compilation results reported by the target emacs. For that step I point at the result of "make archive". I consider this the "package compatibility testing" use case, where I want to make local changes that, after a make invocation, are made available as installable packages. Running admin/update-archive.sh after each change isn't what I want, because it does things like trying to pull elpa.git. > The purpose of the "in-place installation" method is so that those > packages behave kind of like Emacs's bundled packages (except that you > need to add the "packages" dir to your package-directory-list, so they > won't appear in "emacs -Q" and after "cd .../emacs; make" you also have > to do "cd .../elpa; make"). Just like Emacs's bundled packages, you > don't need to "package-install" them, or even to choose which ones > you want. OK. I haven't been using that (other than as a test of byte compilation against the default Emacs) since I've been doing Excorporate feature development in a standalone git repository. But I may use this work flow once I start doing feature development in elpa.git. For now I'm only doing packaging/compatibility work. Speaking of which, I'd like to push the attached patch set, which makes externals deletion safer and adds the first two core packages to GNU ELPA (ntlm and soap-client). Look OK? Thanks, Thomas [-- Attachment #2: add-two-core-packages-1.tar.gz --] [-- Type: application/gzip, Size: 3988 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [elpa] master 872014e: Prevent accidental deletion of .git 2015-11-27 6:58 ` Thomas Fitzsimmons @ 2015-11-27 15:27 ` Stefan Monnier 0 siblings, 0 replies; 17+ messages in thread From: Stefan Monnier @ 2015-11-27 15:27 UTC (permalink / raw) To: Thomas Fitzsimmons; +Cc: Fabián E. Gallina, emacs-devel > But for byte-compilation testing, I do a package-install in each target > emacs, and check the byte-compilation results reported by the target > emacs. For that step I point at the result of "make archive". That sounds excruciating. Why not do: cd .../elpa rm **/*.elc make EMACS="emacs-23.1 --batch" Admittedly, your system is slightly closer to what the end-user will do, so it may give slightly more precise results in some cases. > Speaking of which, I'd like to push the attached patch set, which makes > externals deletion safer and adds the first two core packages to GNU > ELPA (ntlm and soap-client). Look OK? [ Better just attach them so they're easier to view in the MUA. ] Looks fine to me, thanks, Stefan ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-11-27 15:27 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20151109013124.17711.29422@vcs.savannah.gnu.org> [not found] ` <E1ZvbIq-0004cL-VV@vcs.savannah.gnu.org> 2015-11-09 1:58 ` [elpa] master 872014e: Prevent accidental deletion of .git Stefan Monnier 2015-11-09 4:49 ` Thomas Fitzsimmons 2015-11-09 5:03 ` Stefan Monnier 2015-11-09 16:07 ` Eli Zaretskii 2015-11-09 16:38 ` Artur Malabarba 2015-11-09 16:45 ` Eli Zaretskii 2015-11-09 20:20 ` Stefan Monnier 2015-11-18 14:39 ` Thomas Fitzsimmons 2015-11-18 15:10 ` Stefan Monnier 2015-11-18 16:44 ` Stephen Leake 2015-11-18 17:35 ` Eli Zaretskii 2015-11-26 3:08 ` Thomas Fitzsimmons 2015-11-26 3:55 ` Stefan Monnier 2015-11-26 14:34 ` Thomas Fitzsimmons 2015-11-26 15:42 ` Stefan Monnier 2015-11-27 6:58 ` Thomas Fitzsimmons 2015-11-27 15:27 ` Stefan Monnier
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.