Re: [elpa] master 580a1b6: Respect .elpaignore when compiling and optimize compilation.

Re: [elpa] master 580a1b6: Respect .elpaignore when compiling and optimize compilation.

[Stefan Monnier]

> +              tar -ch $$pt/*.el --no-recursion		\
> +                  --exclude-vcs -X "$${pt}/.elpaignore"	\
> +                | tar --list;				\

I think we can skip the second tar with something like
"tar -cvhf /dev/null 2>&1".

But I have a question: are we sure that tar will always apply the -X
"$${pt}/.elpaignore" patterns to the files explicitly mentioned on the
command line?


        Stefan

Re: [elpa] master 580a1b6: Respect .elpaignore when compiling and optimize compilation.

[Jackson Ray Hamilton]

> I think we can skip the second tar with something like
> "tar -cvhf /dev/null 2>&1".

I tried this before, and again just now, but it does not cause make to
execute at a significantly different speed on my machine.  Is the effect
more noticeable on yours?

> But I have a question: are we sure that tar will always apply the -X
> "$${pt}/.elpaignore" patterns to the files explicitly mentioned on the
> command line?

Good point.  I suppose not in this instance, where I get no output:

$ mkdir -p a/a
$ touch a/b.el a/a/b.el
$ echo 'a/b.el' > a/.elpaignore
$ tar -ch a/*.el --no-recursion --exclude-vcs -X a/.elpaignore \
  | tar --list

Since .elpaignore is inside the project directory, I assume that we
should be treating .elpaignore as matching relative to inside the
project root.  We should cd into the project directory first, so we
don't accidentally match against the project's directory's name, too.

$ cd a
$ tar -ch *.el --no-recursion --exclude-vcs -X .elpaignore | tar --list

The above command correctly yields "b.el".

Did you have any other cases in mind?

On 12/23/2016 03:49 PM, Stefan Monnier wrote:
>> +              tar -ch $$pt/*.el --no-recursion		\
>> +                  --exclude-vcs -X "$${pt}/.elpaignore"	\
>> +                | tar --list;				\
> 
> I think we can skip the second tar with something like
> "tar -cvhf /dev/null 2>&1".
> 
> But I have a question: are we sure that tar will always apply the -X
> "$${pt}/.elpaignore" patterns to the files explicitly mentioned on the
> command line?
> 
> 
>         Stefan
> 

Re: [elpa] master 580a1b6: Respect .elpaignore when compiling and optimize compilation.

[Stefan Monnier]

>> I think we can skip the second tar with something like
>> "tar -cvhf /dev/null 2>&1".
> I tried this before, and again just now, but it does not cause make to
> execute at a significantly different speed on my machine.  Is the effect
> more noticeable on yours?

I haven't looked at the performance side of this.  It just seemed simpler.

> $ cd a
> $ tar -ch *.el --no-recursion --exclude-vcs -X .elpaignore | tar --list
>
> The above command correctly yields "b.el".
>
> Did you have any other cases in mind?

No, I was rather wondering if this behavior is documented or just accidental.


        Stefan

Re: [elpa] master 580a1b6: Respect .elpaignore when compiling and optimize compilation.

[Jackson Ray Hamilton]

[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]

> I haven't looked at the performance side of this.  It just seemed simpler.

I say we keep it as-is, as my reading of the current commands is simpler
than the way I read this alternative.  Compare the way I read it: "Make
a tar archive and list its contents."  Versus: "Execute tar with verbose
output, which happens to only include the files packaged, also that
output needs to be redirected from stderr to stdout for make to process
it, also bury the archive contents in /dev/null so as not to interfere
with the redirected verbose output."

> No, I was rather wondering if this behavior is documented or just accidental.

Although I do not see an example of an explicitly mentioned file
argument used with an exclusion flag in the tar manual, the language of
the manual suggests that this should work:

>  `--exclude-from=file'
> `-X file'
> 
>     Causes tar to ignore files that match the patterns listed in file. 

"Files" ought to include explicitly mentioned files.

Also, I've attached a patch for the "pwd-based exclusions" issue I
mentioned in my last email.  I checked the timing on it, and it only
reduces the speed of "make" with nothing to do by about 50ms.  Does it
look good to you?

Jackson

On 12/25/2016 08:16 PM, Stefan Monnier wrote:
>>> I think we can skip the second tar with something like
>>> "tar -cvhf /dev/null 2>&1".
>> I tried this before, and again just now, but it does not cause make to
>> execute at a significantly different speed on my machine.  Is the effect
>> more noticeable on yours?
> 
> I haven't looked at the performance side of this.  It just seemed simpler.
> 
>> $ cd a
>> $ tar -ch *.el --no-recursion --exclude-vcs -X .elpaignore | tar --list
>>
>> The above command correctly yields "b.el".
>>
>> Did you have any other cases in mind?
> 
> No, I was rather wondering if this behavior is documented or just accidental.
> 
> 
>         Stefan
> 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Change-to-package-directory-before-checking-.elpaign.patch --]
[-- Type: text/x-patch; name="0001-Change-to-package-directory-before-checking-.elpaign.patch", Size: 1616 bytes --]

From e05054c3f9b6cf6ee0dbeafd0438b562f06afc64 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Mon, 26 Dec 2016 11:25:21 -0800
Subject: [PATCH] Change to package directory before checking .elpaignore
 exclusions.

If we don't do this, the package's directory's name is included in the file
names that are checked for .elpaignore matches.  .elpaignore matches should be
assumed to occur inside the package's directory.
---
 GNUmakefile | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/GNUmakefile b/GNUmakefile
index 6d57fae..8edb2f8 100644
--- a/GNUmakefile
+++ b/GNUmakefile
@@ -150,13 +150,18 @@ $(foreach al, $(autoloads), $(eval $(call RULE-srcdeps, $(al))))
 included_els := $(shell \
   for pt in packages/*; do				\
       if [ -d $$pt ]; then				\
-          if [ -f "$${pt}/.elpaignore" ]; then		\
-              tar -ch $$pt/*.el --no-recursion		\
-                  --exclude-vcs -X "$${pt}/.elpaignore"	\
+          prev=$$(pwd);					\
+          cd $$pt;					\
+          if [ -f .elpaignore ]; then			\
+              tar -ch *.el --no-recursion		\
+                  --exclude-vcs -X .elpaignore		\
                 | tar --list;				\
           else						\
-              ls -1 $$pt/*.el;				\
-          fi;						\
+              ls -1 *.el;				\
+          fi | while read line; 			\
+                   do echo "$${pt}/$${line}"; 		\
+               done;					\
+          cd $$prev;					\
       fi;						\
   done)
 els := $(call FILTER-nonsrc, $(included_els))
-- 
2.1.4

Re: [elpa] master 580a1b6: Respect .elpaignore when compiling and optimize compilation.

[Stefan Monnier]

>> I haven't looked at the performance side of this.  It just seemed simpler.
> I say we keep it as-is, as my reading of the current commands is simpler
> than the way I read this alternative.  Compare the way I read it: "Make
> a tar archive and list its contents."  Versus: "Execute tar with verbose
> output, which happens to only include the files packaged, also that
> output needs to be redirected from stderr to stdout for make to process
> it, also bury the archive contents in /dev/null so as not to interfere
> with the redirected verbose output."

Fair enough.

> Also, I've attached a patch for the "pwd-based exclusions" issue I
> mentioned in my last email.  I checked the timing on it, and it only
> reduces the speed of "make" with nothing to do by about 50ms.  Does it
> look good to you?

Looks OK.  BTW, I saw that we could do

    included_els := $(shell tar -cvhf /dev/null --exclude-ignore=.elpaignore \
                                --exclude-vcs packages 2>&1 | grep '\.el$$')

except that --exclude-ignore is a fairly recent addition to GNU tar, so
it doesn't even work in Debian stable.


        Stefan