[PATCH 1 of 2] largefiles: fix rename (Issue 3093)

Matt Mackall mpm at selenic.com
Tue Nov 29 14:49:02 CST 2011


On Tue, 2011-11-29 at 16:15 +0100, Na'Tosha Bard wrote:
> # HG changeset patch
> # User Na'Tosha Bard <natosha at unity3d.com>
> # Date 1322579324 -3600
> # Node ID dfe717266e728cb2f780899ad218f3a4fa2a5ee9
> # Parent  ad686c818e1c7d5ed0335327321003ba8ea04664
> largefiles: fix rename (Issue 3093)

Note that hgbot is picky as computers are often wont to be and thus so
am I by proxy: that must be '(issue3093)'. Lowercase, no space.

> diff -r ad686c818e1c -r dfe717266e72 hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py	Fri Nov 25 02:11:12 2011 +0100
> +++ b/hgext/largefiles/overrides.py	Tue Nov 29 16:08:44 2011 +0100
> @@ -389,17 +389,18 @@
>              lfdirstate = lfutil.openlfdirstate(ui, repo)
>              for (src, dest) in copiedfiles:
>                  if lfutil.shortname in src and lfutil.shortname in dest:
> -                    srclfile = src.replace(lfutil.shortname, '')
> -                    destlfile = dest.replace(lfutil.shortname, '')

This is not robust enough. It'll mangle anything that happens to have
".hglf" in it, which means we'll eventually get a weird bug report where
we go back and forth five times until the user finally reveals the
secret filename they've been hiding from us and we go "aha, perhaps it's
a largefiles bug, why would you name a file that, what is wrong with
you?"

..which means breaking down the path components with os.path.dirname and
friends.

> +                    substr = repo.root + "/" + lfutil.shortname + "/"

We should probably use os.path.join or repo.wjoin. A helper function
like lfjoin is probably warranted. Did you test this on a system where
the separator is "\", by any chance?

> +                    srclfile = src.replace(substr, '')
> +                    destlfile = dest.replace(substr, '')
>                      destlfiledir = os.path.dirname(destlfile) or '.'
>                      if not os.path.isdir(destlfiledir):
>                          os.makedirs(destlfiledir)
>                      if rename:
> -                        os.rename(srclfile, destlfile)
> -                        lfdirstate.remove(repo.wjoin(srclfile))
> +                        os.rename(repo.wjoin(srclfile), repo.wjoin(destlfile))
> +                        lfdirstate.remove(srclfile)
>                      else:
>                          util.copyfile(srclfile, destlfile)
> -                    lfdirstate.add(repo.wjoin(destlfile))
> +                    lfdirstate.add(destlfile)
>              lfdirstate.write()
>          except util.Abort, e:
>              if str(e) != 'no files to copy':
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list