[Commented On] D9698: shelve: trust caller of shelvedfile.opener() to check that the file exists

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D9698#146566 <https://phab.mercurial-scm.org/D9698#146566>, @joerg.sonnenberger wrote:
  
  > Big question for me would be what to do with junk entries. I would make "hg shelve -l" list them with a note to remove them manually, but otherwise reject interacting with them with shelve or unshelve. If that is the acceptable before, it seems better to adjust (and test) the semantic tightening first and then refactor the logic. If the intention is to allow removing the junk with "hg shelve -d", it seems easier to refactor first and adjust the behavior afterwards.
  
  I consider it a new feature to better support corrupt entries. For example, before this series:
  
    # Only .hg file does not get listed, but can be deleted (moved to backup).
    $ touch .hg/shelved/junk.hg
    $ hg shelve -l | grep junk
    $ hg shelve -d junk
    $ ls .hg/shelved/junk.hg
    ls: cannot access '.hg/shelved/junk.hg': No such file or directory
    
    # Only .patch file gets listed, but then `hg unshelve` says it doesn't exist. It can be successfully deleted.
    $ touch .hg/shelved/junk.patch
    $ hg shelve -l | grep junk
    junk            (5s ago)
    $ hg unshelve
    unshelving change 'junk'
    abort: shelved change 'junk' not found
    $ hg shelve -d junk
    $ hg shelve -l | grep junk
    <empty>
    
    # Other files in the directory lead to a crash and the user cannot recover without manually deleting the file.
    $ touch .hg/shelved/junk
    $ hg shelve -l
    [...]
    ValueError: not enough values to unpack (expected 2, got 1)
    $ hg shelve -d junk
    abort: shelved change 'junk' not found
  
  I suspect code I'm removing in this patch was added before the higher-level check that we now have have (search for "not found" in the file). I don't think the intent was ever to support corrupt `.hg/shelved/` directories.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D9698/new/

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

To: martinvonz, #hg-reviewers
Cc: joerg.sonnenberger, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20210108/e4f8719a/attachment-0002.html>