This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11340

clone: properly create target directories during local clone (issue6581)
ClosedPublic

Authored by marmoute on Aug 25 2021, 5:41 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG132525ead0db: clone: properly create target directories during local clone (issue6581)
rHGad31e755a7c6: clone: properly create target directories during local clone (issue6581)
Summary

The store encoding was mudding the water. This lead to local clone crashing for
file with long filename as their destination directory needed to be encoded.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

marmoute created this revision.Aug 25 2021, 5:41 AM
marmoute added a comment.Aug 25 2021, 5:57 AM

It need minor fixing for a couple of test output. Update coming.

marmoute planned changes to this revision.Aug 25 2021, 6:09 AM
Alphare added a subscriber: Alphare.Aug 25 2021, 6:21 AM
Alphare added inline comments.
mercurial/streamclone.py
824–828

Not 100% sure what "put them off" means in this context

marmoute updated this revision to Diff 30015.Aug 25 2021, 6:32 AM
baymax updated this revision to Diff 30016.Aug 25 2021, 8:09 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)
⚠ This patch is intended for stable ⚠

Alphare added a comment.Aug 25 2021, 8:50 AM

Looking at https://phab.mercurial-scm.org/D11340?vs=30015&id=30016#toc, it looks like Baymax refreshed with the older diff, since the older CI run took a lot more time to finish than the correct one.

This is because I think f6879956a386 made it so we still phabupdate when the tests break, which seems like a very dangerous thing. @marmoute am I wrong?

For this series, I see that 7b5827f1a199 is the correct one (I have it from the Heptapod repo), so I'll take that directly.

Alphare accepted this revision.Aug 25 2021, 9:04 AM

Accepting the "previous" revision as per my previous comment

This revision is now accepted and ready to land.Aug 25 2021, 9:04 AM
marmoute added a comment.Aug 26 2021, 3:46 AM
In D11340#173949, @Alphare wrote:

Looking at https://phab.mercurial-scm.org/D11340?vs=30015&id=30016#toc, it looks like Baymax refreshed with the older diff, since the older CI run took a lot more time to finish than the correct one.
This is because I think f6879956a386 made it so we still phabupdate when the tests break, which seems like a very dangerous thing. @marmoute am I wrong?

Urg, probably. Let me follow up with a fix.

Revision Contents
Changeset List

PathPackages
Mmercurial/streamclone.py (8 lines)
Mtests/test-clone-stream.t (18 lines)

Diff 30021

mercurial/streamclone.py

hardlink[0] = False hardlink[0] = False
progress.topic = _(b'copying') progress.topic = _(b'copying')
for k, path, size in entries: for k, path, size in entries:
src_vfs = src_vfs_map[k] src_vfs = src_vfs_map[k]
dst_vfs = dst_vfs_map[k] dst_vfs = dst_vfs_map[k]
src_path = src_vfs.join(path) src_path = src_vfs.join(path)
dst_path = dst_vfs.join(path) dst_path = dst_vfs.join(path)
dirname = dst_vfs.dirname(path) # We cannot use dirname and makedirs of dst_vfs here because the store
if not dst_vfs.exists(dirname): # encoding confuses them. See issue 6581 for details.
dst_vfs.makedirs(dirname) dirname = os.path.dirname(dst_path)
if not os.path.exists(dirname):
util.makedirs(dirname)
AlphareUnsubmitted
Not Done

Not 100% sure what "put them off" means in this context

Alphare: Not 100% sure what "put them off" means in this context
dst_vfs.register_file(path) dst_vfs.register_file(path)
# XXX we could use the #nb_bytes argument. # XXX we could use the #nb_bytes argument.
util.copyfile( util.copyfile(
src_path, src_path,
dst_path, dst_path,
hardlink=hardlink[0], hardlink=hardlink[0],
no_hardlink_cb=copy_used, no_hardlink_cb=copy_used,
check_fs_hardlink=False, check_fs_hardlink=False,

tests/test-clone-stream.t

Check local clone Check local clone
================== ==================
The logic is close enough of uncompressed. The logic is close enough of uncompressed.
This is present here to reuse the testing around file with "special" names. This is present here to reuse the testing around file with "special" names.
$ hg clone server local-clone $ hg clone server local-clone
updating to branch default (missing-correct-output !) updating to branch default
1088 files updated, 0 files merged, 0 files removed, 0 files unresolved (missing-correct-output !) 1088 files updated, 0 files merged, 0 files removed, 0 files unresolved
abort: $ENOENT$: '$TESTTMP/local-clone/.hg/store/dh/containe/isam-bui/bazel-coverage-generator-sandboxfs-compatibility-0758e3e4d94041277bcd011e1d54c247523c124b4a325686.i' (known-bad-output !)
[255]
Check that the clone went well Check that the clone went well
$ hg verify -R local-clone $ hg verify -R local-clone
checking changesets (missing-correct-output !) checking changesets
checking manifests (missing-correct-output !) checking manifests
crosschecking files in changesets and manifests (missing-correct-output !) crosschecking files in changesets and manifests
checking files (missing-correct-output !) checking files
checked 3 changesets with 1088 changes to 1088 files (missing-correct-output !) checked 3 changesets with 1088 changes to 1088 files
abort: repository local-clone not found (known-bad-output !)
[255]
Check uncompressed Check uncompressed
================== ==================
Cannot stream clone when server.uncompressed is set Cannot stream clone when server.uncompressed is set
$ get-with-headers.py $LOCALIP:$HGPORT '?cmd=stream_out' $ get-with-headers.py $LOCALIP:$HGPORT '?cmd=stream_out'
200 Script output follows 200 Script output follows