From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rasmus Subject: Re: [PATCH] org-attach.el: Fetch attachments from git annex Date: Mon, 25 Jan 2016 22:19:56 +0100 Message-ID: <87lh7dz79f.fsf@gmx.us> References: <568b532e.d111620a.b25a8.ffffbb7c@mx.google.com> <87poxg8s22.fsf@kyleam.com> <568c6aaa.c345620a.7f4da.6359@mx.google.com> <56a5b193.ca77420a.1551e.667c@mx.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:48290) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aNoYR-0000ay-Fm for emacs-orgmode@gnu.org; Mon, 25 Jan 2016 16:20:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aNoYO-0003qJ-9J for emacs-orgmode@gnu.org; Mon, 25 Jan 2016 16:20:07 -0500 Received: from plane.gmane.org ([80.91.229.3]:55906) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aNoYO-0003po-1H for emacs-orgmode@gnu.org; Mon, 25 Jan 2016 16:20:04 -0500 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1aNoYM-0003Uk-48 for emacs-orgmode@gnu.org; Mon, 25 Jan 2016 22:20:02 +0100 Received: from x4db0ffa9.dyn.telefonica.de ([77.176.255.169]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 25 Jan 2016 22:20:02 +0100 Received: from rasmus by x4db0ffa9.dyn.telefonica.de with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 25 Jan 2016 22:20:02 +0100 List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: emacs-orgmode@gnu.org Hi Erik, Thanks for the patch. Note that between git-annex v6 and annex.largefiles most of the checking is unnecessary. In my opinion it would be much more useful to start ripping out annex specific code, though the automatic fetching should be added. Erik Hetzner writes: > +(defun org-attach-use-annex () > + "Return non-nil if git annex can be used." > + (let ((git-dir (vc-git-root (expand-file-name org-attach-directory)))) > + (and org-attach-git-annex-cutoff > + (or (file-exists-p (expand-file-name "annex" git-dir)) > + (file-exists-p (expand-file-name ".git/annex" git-dir)))))) Seems fine, but I wonder if it wouldn’t be better to check the exit code of say an annex command and relies on its checking. E.g. on my system (zerop (shell-command "cd ~/annex/doc.annex/ && git annex info --fast" nil)) => t (zerop (shell-command "cd ~/src/code/org-mode && git annex info --fast" nil )) => nil > +(defun org-attach-annex-fetch-maybe (path) > + "Fetch PATH from git annex if necessary." > + (when (and (org-attach-use-annex) > + (not (string-equal "found" > + (shell-command-to-string > + (format "git annex find --format=found --in=here %s" (shell-quote-argument path)))))) > + (message "Fetching \"%s\" using git annex." path) > + (call-process "git" nil nil nil "annex" "get" path))) AFAIK annex will check if get should get anything. If that’s correct, I’d prefer to just rely on however git annex get checks files. > + (dolist (new-or-modified > + (split-string > + (shell-command-to-string > + "git ls-files -zmo --exclude-standard") "\0" t)) When I run this command in one of my annex repos I get: fatal: This operation must be run in a work tree Maybe you are assuming indirect mode? > + (if (and (org-attach-use-annex) wouldn’t it be better to bind the return value of (org-attach-use-annex) rather than call it once per file to be added? Since there’s no dir argument, I guess it won’t change. > + (>= (nth 7 (file-attributes new-or-modified)) > + org-attach-git-annex-cutoff)) If people want this they can use annex.largefiles. Reimplementing annex.largefiles is not within Org’s domain. It’s even more the case with annex v6. > + (call-process "git" nil nil nil "annex" "add" new-or-modified) > + (call-process "git" nil nil nil "add" new-or-modified)) In git annex v6 you don’t need to call "git annex add" (but can). In git annex v5 you don’t need to call "git add". To be compatible between with both v5 and v6 you can just call "git annex add", I guess. > +++ b/testing/lisp/test-org-attach.el > @@ -0,0 +1,81 @@ > +;;; test-org-attach.el --- Tests for Org Attach > +;; > +;; Copyright (c) 2016 Erik Hetzner > +;; Authors: Erik Hetzner I did not check this part. Thanks, Rasmus -- Not everything that goes around comes back around, you know