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.

Download Raw Diff

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

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 25 2021, 5:41 AM

Herald added a subscriber: mercurial-patches. · View Herald Transcript

Associated CI run still in progress: https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/26248

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

That run should be fine (but still in progress) : https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/26249

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

comment updated: latest run is here https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/26255

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

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.

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 commit: rHGad31e755a7c6: clone: properly create target directories during local clone (issue6581).Aug 25 2021, 9:07 AM

Closed by commit rHGad31e755a7c6: clone: properly create target directories during local clone (issue6581) (authored by marmoute). · Explain Why

This revision was automatically updated to reflect the committed changes.

marmoute added a commit: rHG132525ead0db: clone: properly create target directories during local clone (issue6581).Aug 25 2021, 9:15 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

			Path	Packages
M			mercurial/streamclone.py (8 lines)
M			tests/test-clone-stream.t (18 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	30010		Aug 25 2021, 5:41 AM	★	★
Diff 2	30015		Aug 25 2021, 6:32 AM	★	★
Diff 3	30016		Aug 25 2021, 8:09 AM	★	★
Diff 4	30021	rHGad31e755a7c6be4d1759d7c27c08b001ba3b4e80	Aug 25 2021, 5:32 AM	★	★

Status	Author	Revision
Closed	marmoute	D11340 clone: properly create target directories during local clone (issue6581)
Closed	marmoute	D11339 clone: add a file demonstrating issue6581 in test-clone-stream.t
Closed	marmoute	D11338 clone: verify the local clone in test-clone-stream.t
Closed	marmoute	D11337 clone: automatically glob stream clone output in test
Closed	marmoute	D11336 clone: fix a comment in test-clone-stream.t

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