Patchwork D9207: record: when backing up, avoid generating very long filenames

login
register
mail settings
Submitter phabricator
Date Oct. 14, 2020, 10:12 p.m.
Message ID <differential-rev-PHID-DREV-3pcblkivziqeelyjt7qr-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/47466/
State Superseded
Headers show

Comments

phabricator - Oct. 14, 2020, 10:12 p.m.
spectral created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  If the original file's path is longer than the individual filename maximum
  length (256 on Linux, I believe?), then this mechanism of "replace slashes with
  underscores" causes an error.
  
  Now, we'll produce just the "basename" of the file, plus some stuff to ensure
  it's unique. This can be potentially confusing for users if there's a file with
  the same name in multiple directories, but I suspect that this is better than
  just breaking.
  
  Example:
  `<reporoot>/a/long/path/to/somefile.txt` used to be backed up as
  `<reporoot>/.hg/record-backups/a_long_path_to_somefile.txt.abcdefgh`, it will
  now be backed up as `<reporoot>/.hg/record-backups/somefile.txt.abcdefgh`
  
  We could do the naive thing (what we were doing before) and have it to doing
  something with either subdirectories
  (`<backuproot>/a/long/path/to/somefile.txt.abcdefgh` or minimize #dirs with
  `<backuproot>/a_long_path/to_somefile.txt.abcdefgh`), prefix-truncated paths
  (such as `<backuproot>/__ath_to_somefile.txt.abcdefgh`, where that `__` elides
  enough to get us under 255 chars (counting the +9 we need to add!)), or
  hash-of-dirname (`<backuproot>/<sha1sum_of_dirname>/somefile.txt.abcdefgh`), but
  ultimately every option felt over engineered and that it would be more likely to
  cause problems than it would be to solve any, especially if it was conditional
  on directory length.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9207

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS




To: spectral, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -558,7 +558,7 @@ 
             # backup continues
             for f in tobackup:
                 fd, tmpname = pycompat.mkstemp(
-                    prefix=f.replace(b'/', b'_') + b'.', dir=backupdir
+                    prefix=os.path.basename(f) + b'.', dir=backupdir
                 )
                 os.close(fd)
                 ui.debug(b'backup %r as %r\n' % (f, tmpname))