Patchwork [1,of,2,V3] archive: raise error.Abort if the file pattern matches no files

login
register
mail settings
Submitter Angel Ezquerra
Date April 13, 2013, 7:45 p.m.
Message ID <47797f2b57b3e4e4512b.1365882318@Angel-PC.localdomain>
Download mbox | patch
Permalink /patch/1294/
State Accepted
Commit 88d1b59f69060a1e929b7db6ac0b29fa2b194e28
Headers show

Comments

Angel Ezquerra - April 13, 2013, 7:45 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1363900155 -3600
#      Thu Mar 21 22:09:15 2013 +0100
# Node ID 47797f2b57b3e4e4512b3c637d3082a6f7c6dee4
# Parent  8c0a7eeda06d2773ec92b14527280db3e0167588
archive: raise error.Abort if the file pattern matches no files

Note that we could raise this exception even if no pattern were specified, but
the revision contained no files. However this should not happen in practice
since in that case commands.py/archive would exit earlier with an "no working
directory: please specify a revision" error message instead.
Bryan O'Sullivan - April 13, 2013, 9:57 p.m.
On Sat, Apr 13, 2013 at 12:45 PM, Angel Ezquerra
<angel.ezquerra@gmail.com>wrote:

> +    if total:
> +        files.sort()
> +        repo.ui.progress(_('archiving'), 0, unit=_('files'), total=total)
> +        for i, f in enumerate(files):
> +            ff = ctx.flags(f)
> +            write(f, 'x' in ff and 0755 or 0644, 'l' in ff, ctx[f].data)
> +            repo.ui.progress(_('archiving'), i + 1, item=f,
> +                             unit=_('files'), total=total)
> +        repo.ui.progress(_('archiving'), None)
>

Why don't you leave out the test of total here and leave this block
untouched.
Angel Ezquerra - April 13, 2013, 11:58 p.m.
On Apr 13, 2013 11:58 PM, "Bryan O'Sullivan" <bos@serpentine.com> wrote:
>
>
> On Sat, Apr 13, 2013 at 12:45 PM, Angel Ezquerra <angel.ezquerra@gmail.com>
wrote:
>>
>> +    if total:
>> +        files.sort()
>> +        repo.ui.progress(_('archiving'), 0, unit=_('files'),
total=total)
>> +        for i, f in enumerate(files):
>> +            ff = ctx.flags(f)
>> +            write(f, 'x' in ff and 0755 or 0644, 'l' in ff, ctx[f].data)
>> +            repo.ui.progress(_('archiving'), i + 1, item=f,
>> +                             unit=_('files'), total=total)
>> +        repo.ui.progress(_('archiving'), None)
>
>
> Why don't you leave out the test of total here and leave this block
untouched.

I hesitated a bit with that. I thought that it might be a bit weird to get
a message saying "archiving 0 files" and then getting an exception saying
that "no files match the pattern".

That being said I don't feel strongly about this. I will change it as you
suggest if you think that'd be best.

Angel

Patch

diff --git a/mercurial/archival.py b/mercurial/archival.py
--- a/mercurial/archival.py
+++ b/mercurial/archival.py
@@ -13,6 +13,7 @@ 
 import cStringIO, os, tarfile, time, zipfile
 import zlib, gzip
 import struct
+import error
 
 # from unzip source code:
 _UNX_IFREG = 0x8000
@@ -288,20 +289,25 @@ 
         files = [f for f in ctx.manifest().keys() if matchfn(f)]
     else:
         files = ctx.manifest().keys()
-    files.sort()
     total = len(files)
-    repo.ui.progress(_('archiving'), 0, unit=_('files'), total=total)
-    for i, f in enumerate(files):
-        ff = ctx.flags(f)
-        write(f, 'x' in ff and 0755 or 0644, 'l' in ff, ctx[f].data)
-        repo.ui.progress(_('archiving'), i + 1, item=f,
-                         unit=_('files'), total=total)
-    repo.ui.progress(_('archiving'), None)
+    if total:
+        files.sort()
+        repo.ui.progress(_('archiving'), 0, unit=_('files'), total=total)
+        for i, f in enumerate(files):
+            ff = ctx.flags(f)
+            write(f, 'x' in ff and 0755 or 0644, 'l' in ff, ctx[f].data)
+            repo.ui.progress(_('archiving'), i + 1, item=f,
+                             unit=_('files'), total=total)
+        repo.ui.progress(_('archiving'), None)
 
     if subrepos:
         for subpath in sorted(ctx.substate):
             sub = ctx.sub(subpath)
             submatch = matchmod.narrowmatcher(subpath, matchfn)
-            sub.archive(repo.ui, archiver, prefix, submatch)
+            total += sub.archive(repo.ui, archiver, prefix, submatch)
+
+    if total == 0:
+        raise error.Abort(_('no files match the archive pattern'))
 
     archiver.done()
+    return total
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -392,6 +392,7 @@ 
             ui.progress(_('archiving (%s)') % relpath, i + 1,
                         unit=_('files'), total=total)
         ui.progress(_('archiving (%s)') % relpath, None)
+        return total
 
     def walk(self, match):
         '''
@@ -483,14 +484,15 @@ 
     @annotatesubrepoerror
     def archive(self, ui, archiver, prefix, match=None):
         self._get(self._state + ('hg',))
-        abstractsubrepo.archive(self, ui, archiver, prefix, match)
-
+        total = abstractsubrepo.archive(self, ui, archiver, prefix, match)
         rev = self._state[1]
         ctx = self._repo[rev]
         for subpath in ctx.substate:
             s = subrepo(ctx, subpath)
             submatch = matchmod.narrowmatcher(subpath, match)
-            s.archive(ui, archiver, os.path.join(prefix, self._path), submatch)
+            total += s.archive(
+                ui, archiver, os.path.join(prefix, self._path), submatch)
+        return total
 
     @annotatesubrepoerror
     def dirty(self, ignoreupdate=False):
@@ -1271,9 +1273,10 @@ 
                 os.remove(path)
 
     def archive(self, ui, archiver, prefix, match=None):
+        total = 0
         source, revision = self._state
         if not revision:
-            return
+            return total
         self._fetch(source, revision)
 
         # Parse git's native archive command.
@@ -1294,9 +1297,11 @@ 
                 data = tar.extractfile(info).read()
             archiver.addfile(os.path.join(prefix, self._path, info.name),
                              info.mode, info.issym(), data)
+            total += 1
             ui.progress(_('archiving (%s)') % relpath, i + 1,
                         unit=_('files'))
         ui.progress(_('archiving (%s)') % relpath, None)
+        return total
 
 
     @annotatesubrepoerror
diff --git a/tests/test-archive.t b/tests/test-archive.t
--- a/tests/test-archive.t
+++ b/tests/test-archive.t
@@ -289,6 +289,16 @@ 
   *-----* (glob)
   \s*147\s+2 files (re)
 
+show an error when a provided pattern matches no files
+
+  $ hg archive -I file_that_does_not_exist.foo ../empty.zip
+  abort: no files match the archive pattern
+  [255]
+
+  $ hg archive -X * ../empty.zip
+  abort: no files match the archive pattern
+  [255]
+
   $ cd ..
 
 issue3600: check whether "hg archive" can create archive files which