| Submitter | Levi Bard |
|---|---|
| Date | Dec. 10, 2012, 2:01 p.m. |
| Message ID | <fcf5fda9138ea94bce52.1355148091@retina-monoteam> |
| Download | mbox | patch |
| Permalink | /patch/39/ |
| State | Superseded |
| Commit | 7e2b9f6a2cd043966fca50a29a9867fb12387a22 |
| Headers | show |
Comments
On Dec 10, 2012, at 8:01 AM, Levi Bard wrote: > # HG changeset patch > # User Levi Bard <levi at unity3d.com> > # Date 1355147922 -3600 > # Node ID fcf5fda9138ea94bce52a03c7af618904b50f16c > # Parent 40374059d227850ec2f5fb4f21a1b619136e2a6a > largefiles: Commit directories that only contain largefiles (issue3548) > > If we pass a directory to commit whose only commitable files > are largefiles, the core commit code aborts before finding > the largefiles. > So we do the following: > For directories that only have largefiles as matches, > we explicitly add the largefiles to the matchlist and remove > the directory. > In other cases, we leave the match list unmodified. Oof, the wrapping here makes my eyes burn. Whitespace and bullets (asterisks or hyphens) are your friends. :) Also, the first sentence of the long description isn't clear if it's describing the state before or after this patch. Code looks superficially good, but I'm willfully unfamiliar with the largefiles code. > diff -r 40374059d227 -r fcf5fda9138e tests/test-largefiles.t > --- a/tests/test-largefiles.t Tue Nov 27 22:24:02 2012 +0100 > +++ b/tests/test-largefiles.t Mon Dec 10 14:58:42 2012 +0100 > @@ -345,6 +345,59 @@ > A sub2/large6 > A sub2/large7 > > +Committing directories containing only large files. +1 for adding a test without adding an `hg init`. pacem in terris / ??? / ?????? / ????????? / ?? Kevin R. Bullock
On Mon, 10 Dec 2012 15:01:31 +0100, Levi Bard wrote: > # HG changeset patch > # User Levi Bard <levi at unity3d.com> > # Date 1355147922 -3600 > # Node ID fcf5fda9138ea94bce52a03c7af618904b50f16c > # Parent 40374059d227850ec2f5fb4f21a1b619136e2a6a > largefiles: Commit directories that only contain largefiles (issue3548) ^ One small issue- this command used to get rejected: $ hg ci -m "standin" .hglf abort: .hglf: no match under directory! [255] But now it succeeds, sort of: $ hg ci -m "standin" .hglf Invoking status precommit hook M large3 A large5 A sub2/large6 A sub2/large7 A z/y/x/m/normal $ hg st M large3 A large5 A sub2/large6 A sub2/large7 A z/y/x/m/normal (No error/exit failure, but no apparent change either, in which case the exit code should be 1. I'm not sure which of the before and after is the proper behavior.) A question below, but otherwise looks good. > diff -r 40374059d227 -r fcf5fda9138e hgext/largefiles/reposetup.py > --- a/hgext/largefiles/reposetup.py Tue Nov 27 22:24:02 2012 +0100 > +++ b/hgext/largefiles/reposetup.py Mon Dec 10 14:58:42 2012 +0100 ... > @@ -467,6 +463,60 @@ > return super(lfilesrepo, self).push(remote, force, revs, > newbranch) > > + def _subdirlfs(self, files, lfiles): > + ''' > + Adjust matched file list > + If we pass a directory to commit whose only commitable files > + are largefiles, the core commit code aborts before finding > + the largefiles. > + So we do the following: > + For directories that only have largefiles as matches, > + we explicitly add the largefiles to the matchlist and remove > + the directory. > + In other cases, we leave the match list unmodified. > + ''' > + actualfiles = [] > + dirs = [] > + regulars = [] > + > + for f in files: > + if lfutil.isstandin(f): > + raise util.Abort( > + _('file "%s" is a largefile standin') % f, > + hint=('commit the largefile itself instead')) > + # Scan directories > + if os.path.isdir(self.wjoin(f)): > + dirs.append(f) > + else: > + regulars.append(f) > + > + for f in dirs: > + matcheddir = False > + d = self.dirstate.normalize(f) + '/' > + # Check for matched normal files > + for mf in regulars: > + if self.dirstate.normalize(mf).startswith(d): > + actualfiles.append(f) > + matcheddir = True > + break > + if not matcheddir: > + # If no normal match, manually append > + # any matching largefiles > + for lf in lfiles: > + if self.dirstate.normalize(lf).startswith(d): > + actualfiles.append(lf) > + if not matcheddir: > + actualfiles.append(lfutil.standin(f)) > + matcheddir = True I can't figure out why the previous 3 lines are needed. I commented them out and the test blew up, so they are needed. It appears that all of the largefiles are added to actualfiles, and at the same time, only the first corresponding standin (per directory) is. Why are the standins needed at all here, since the largefiles are provided? And since at least one seems to be needed, why aren't all of the standins needed? > + # Nothing in dir, so readd it > + # and let commit reject it > + if not matcheddir: > + actualfiles.append(f) > + > + # Always add normal files > + actualfiles += regulars > + return actualfiles > + > repo.__class__ = lfilesrepo > > def checkrequireslfiles(ui, repo, **kwargs):
> One small issue- this command used to get rejected: I'll revise the patch to address this behavior change. > > + if not matcheddir: > > + actualfiles.append(lfutil.standin(f)) > > + matcheddir = True > > I can't figure out why the previous 3 lines are needed. I commented them > out and the test blew up, so they are needed. It appears that all of the > largefiles are added to actualfiles, and at the same time, only the first > corresponding standin (per directory) is. Why are the standins needed at > all here, since the largefiles are provided? And since at least one > seems to be needed, why aren't all of the standins needed? It's adding the standin for the directory, for consistency.
Patch
diff -r 40374059d227 -r fcf5fda9138e hgext/largefiles/reposetup.py --- a/hgext/largefiles/reposetup.py Tue Nov 27 22:24:02 2012 +0100 +++ b/hgext/largefiles/reposetup.py Mon Dec 10 14:58:42 2012 +0100 @@ -359,11 +359,8 @@ lfdirstate.write() return result - for f in match.files(): - if lfutil.isstandin(f): - raise util.Abort( - _('file "%s" is a largefile standin') % f, - hint=('commit the largefile itself instead')) + lfiles = lfutil.listlfiles(self) + match._files = self._subdirlfs(match.files(), lfiles) # Case 2: user calls commit with specified patterns: refresh # any matching big files. @@ -394,7 +391,6 @@ # standins corresponding to the big files requested by the # user. Have to modify _files to prevent commit() from # complaining "not tracked" for big files. - lfiles = lfutil.listlfiles(self) match = copy.copy(match) origmatchfn = match.matchfn @@ -467,6 +463,60 @@ return super(lfilesrepo, self).push(remote, force, revs, newbranch) + def _subdirlfs(self, files, lfiles): + ''' + Adjust matched file list + If we pass a directory to commit whose only commitable files + are largefiles, the core commit code aborts before finding + the largefiles. + So we do the following: + For directories that only have largefiles as matches, + we explicitly add the largefiles to the matchlist and remove + the directory. + In other cases, we leave the match list unmodified. + ''' + actualfiles = [] + dirs = [] + regulars = [] + + for f in files: + if lfutil.isstandin(f): + raise util.Abort( + _('file "%s" is a largefile standin') % f, + hint=('commit the largefile itself instead')) + # Scan directories + if os.path.isdir(self.wjoin(f)): + dirs.append(f) + else: + regulars.append(f) + + for f in dirs: + matcheddir = False + d = self.dirstate.normalize(f) + '/' + # Check for matched normal files + for mf in regulars: + if self.dirstate.normalize(mf).startswith(d): + actualfiles.append(f) + matcheddir = True + break + if not matcheddir: + # If no normal match, manually append + # any matching largefiles + for lf in lfiles: + if self.dirstate.normalize(lf).startswith(d): + actualfiles.append(lf) + if not matcheddir: + actualfiles.append(lfutil.standin(f)) + matcheddir = True + # Nothing in dir, so readd it + # and let commit reject it + if not matcheddir: + actualfiles.append(f) + + # Always add normal files + actualfiles += regulars + return actualfiles + repo.__class__ = lfilesrepo def checkrequireslfiles(ui, repo, **kwargs): diff -r 40374059d227 -r fcf5fda9138e tests/test-largefiles.t --- a/tests/test-largefiles.t Tue Nov 27 22:24:02 2012 +0100 +++ b/tests/test-largefiles.t Mon Dec 10 14:58:42 2012 +0100 @@ -345,6 +345,59 @@ A sub2/large6 A sub2/large7 +Committing directories containing only largefiles. + + $ mkdir -p z/y/x/m + $ touch z/y/x/m/large1 + $ touch z/y/x/large2 + $ hg add --large z/y/x/m/large1 z/y/x/large2 + $ hg commit -m "Subdir with directory only containing largefiles" z + Invoking status precommit hook + M large3 + A large5 + A sub2/large6 + A sub2/large7 + A z/y/x/large2 + A z/y/x/m/large1 + $ hg rollback --quiet + $ touch z/y/x/m/normal + $ hg add z/y/x/m/normal + $ hg commit -m "Subdir with mixed contents" z + Invoking status precommit hook + M large3 + A large5 + A sub2/large6 + A sub2/large7 + A z/y/x/large2 + A z/y/x/m/large1 + A z/y/x/m/normal + $ hg st + M large3 + A large5 + A sub2/large6 + A sub2/large7 + $ hg rollback --quiet + $ hg revert z/y/x/large2 z/y/x/m/large1 + $ rm z/y/x/large2 z/y/x/m/large1 + $ hg commit -m "Subdir with normal contents" z + Invoking status precommit hook + M large3 + A large5 + A sub2/large6 + A sub2/large7 + A z/y/x/m/normal + $ hg st + M large3 + A large5 + A sub2/large6 + A sub2/large7 + $ hg rollback --quiet + $ hg revert --quiet z + $ hg commit -m "Empty subdir" z + abort: z: no match under directory! + [255] + $ rm -rf z + Test "hg status" with combination of 'file pattern' and 'directory pattern' for largefiles: