Skip to content

Commit 44a642f

Browse files
Revathy Kasipandianfacebook-github-bot
authored andcommitted
slapi: add exact behavior field to committranslateID
Summary: To fix the issue exposed in D73958187 Reviewed By: muirdm Differential Revision: D74024720 fbshipit-source-id: 79d2e174d885669291a9b702258ce16f709117ba
1 parent 45255d3 commit 44a642f

File tree

9 files changed

+137
-8
lines changed

9 files changed

+137
-8
lines changed

eden/mononoke/edenapi_service/src/handlers/commit.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,17 @@ impl SaplingRemoteApiHandler for CommitTranslateId {
10861086
let mut git_ids = Vec::new();
10871087
let mut globalrevs = Vec::new();
10881088

1089+
let lookup_behavior = match request.lookup_behavior.as_deref() {
1090+
Some("exact") => XRepoLookupExactBehaviour::OnlyExactMapping,
1091+
None | Some("equivalent") => XRepoLookupExactBehaviour::WorkingCopyEquivalence,
1092+
Some(behavior) => {
1093+
return Err(HttpError::e400(MononokeError::InvalidRequest(format!(
1094+
"invalid lookup behavior '{behavior}'"
1095+
)))
1096+
.into());
1097+
}
1098+
};
1099+
10891100
for commit in &request.commits {
10901101
match commit {
10911102
CommitId::Hg(hg_id) => hg_ids.push(HgChangesetId::from(*hg_id)),
@@ -1126,7 +1137,7 @@ impl SaplingRemoteApiHandler for CommitTranslateId {
11261137
bs.clone(),
11271138
None,
11281139
XRepoLookupSyncBehaviour::SyncIfAbsent,
1129-
XRepoLookupExactBehaviour::WorkingCopyEquivalence,
1140+
lookup_behavior,
11301141
)
11311142
.await,
11321143
)
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
#
3+
# This software may be used and distributed according to the terms of the
4+
# GNU General Public License found in the LICENSE file in the root
5+
# directory of this source tree.
6+
7+
The test is to demonstrates the importance of exact mapping in cross-repo commit
8+
translation . The test demonstrates how two different commits in fbsoure/www and fbsource/fbcode respectively
9+
can translate to a same www commit without exact mapping enabled
10+
11+
This is a fork of test-cross-repo-commit-sync-live.t that brings the via-extra mode
12+
to be fully able to deal with mapping changes regardless of sync direction. I will
13+
replace that file once fully fixed.
14+
$ export LARGE_REPO_ID=0
15+
$ export SMALL_REPO_ID=1
16+
$ . "${TEST_FIXTURES}/library.sh"
17+
$ . "${TEST_FIXTURES}/library-push-redirector.sh"
18+
19+
$ merge_just_knobs <<EOF
20+
> {
21+
> "bools": {
22+
> "scm/mononoke:cross_repo_skip_backsyncing_ordinary_empty_commits": true
23+
> }
24+
> }
25+
> EOF
26+
27+
-- Init Mononoke thingies
28+
$ create_large_small_repo
29+
Adding synced mapping entry
30+
$ setup_configerator_configs
31+
$ enable_pushredirect 1 false false
32+
$ XREPOSYNC=1 start_large_small_repo
33+
Starting Mononoke server
34+
$ init_local_large_small_clones
35+
36+
-- Start up the sync job in the background
37+
$ mononoke_x_repo_sync_forever $REPOIDSMALL $REPOIDLARGE
38+
39+
Before the change
40+
-- push to a small repo
41+
$ cd "$TESTTMP/small-hg-client"
42+
$ hg up -q master_bookmark
43+
$ mkdir -p non_path_shifting
44+
$ echo a > foo
45+
$ echo b > non_path_shifting/bar
46+
$ hg ci -Aqm "small repo commit"
47+
$ hg push -r . --to master_bookmark -q
48+
$ log
49+
@ small repo commit [public;rev=2;fe2cc102ee42] remote/master_bookmark
50+
51+
o first post-move commit [public;rev=1;11f848659bfc]
52+
53+
o pre-move commit [public;rev=0;fc7ae591de0e]
54+
$
55+
56+
-- wait a little to give sync job some time to catch up
57+
$ wait_for_xrepo_sync 2
58+
$ flush_mononoke_bookmarks
59+
60+
-- check the same commit in the large repo
61+
$ cd "$TESTTMP/large-hg-client"
62+
$ hg pull -q
63+
$ hg up -q master_bookmark
64+
$ log -r master_bookmark
65+
@ small repo commit [public;rev=3;733eb6ff5cba] remote/master_bookmark
66+
67+
~
68+
$ LARGE_REPO_MAPPED_COMMIT=$(hg whereami)
69+
70+
71+
# Make an large-repo-only commit
72+
$ echo "large-repo only change" > fbcodefile2
73+
$ hg add fbcodefile2
74+
$ hg commit -m "large repo only commit"
75+
$ hg push -r . --to master_bookmark -q
76+
$ LARGE_REPO_ONLY_COMMIT=$(hg log -r . -T '{node}')
77+
78+
# Show the initial small commit mapped to large repo and the second commit made in large repo
79+
# point to the same small repo commit
80+
$ x_repo_lookup large-mon small-mon $LARGE_REPO_ONLY_COMMIT
81+
fe2cc102ee42147f9f2f2095f649efe7aa559f0d
82+
$ x_repo_lookup large-mon small-mon $LARGE_REPO_MAPPED_COMMIT
83+
fe2cc102ee42147f9f2f2095f649efe7aa559f0d
84+
# No lookup behavior specified; defaults to None
85+
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i "'large-mon'" -i "'small-mon'"
86+
[{"commit": {"Hg": bin("54ab50dcc6a97355f6ccbe8bdcd64c2ebb7d82d0")},
87+
"translated": {"Hg": bin("fe2cc102ee42147f9f2f2095f649efe7aa559f0d")}}]
88+
# "equivalent" lookup behavior: maps to a commit in the large repository
89+
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i "'large-mon'" -i "'small-mon'" -i "'equivalent'"
90+
[{"commit": {"Hg": bin("54ab50dcc6a97355f6ccbe8bdcd64c2ebb7d82d0")},
91+
"translated": {"Hg": bin("fe2cc102ee42147f9f2f2095f649efe7aa559f0d")}}]
92+
# Explicitly passing None for lookup behavior; maps to a commit in the large repository
93+
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i "'large-mon'" -i "'small-mon'" -i None
94+
[{"commit": {"Hg": bin("54ab50dcc6a97355f6ccbe8bdcd64c2ebb7d82d0")},
95+
"translated": {"Hg": bin("fe2cc102ee42147f9f2f2095f649efe7aa559f0d")}}]
96+
# "exact" lookup behavior: returns only the exact matching commit, otherwise an empty list
97+
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i "'large-mon'" -i "'small-mon'" -i "'exact'"
98+
[]
99+
# Invalid lookup behavior: 'random' string passed as argument
100+
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i "'large-mon'" -i "'small-mon'" -i "'random'"
101+
abort: server responded 400 Bad Request for https://localhost:*/edenapi/large-mon/commit/translate_id: {"message":"invalid request: invalid lookup behavior * (glob)
102+
"x-request-id": "*", (glob)
103+
"content-type": "application/json",
104+
"x-load": "1",
105+
"server": "edenapi_server",
106+
"x-mononoke-host": * (glob)
107+
"date": * (glob)
108+
"content-length": "*", (glob)
109+
}
110+
[255]

eden/mononoke/tests/integration/edenapi/test-edenapi-translate-id.t

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,12 @@ Setup config repo:
9696
e6c6c1f94d10495f8c94d81ae5f125bd89f82b8a
9797
$ LARGE_REPO_ONLY_COMMIT=$(hg log -r . -T '{node}')
9898

99-
# FIXME: SaplingRemoteAPI returns the same small repo commit hash for both LARGE_REPO_MAPPED_COMMIT and LARGE_REPO_ONLY_COMMIT.
99+
# Returns an empty array [] for exact lookup behavior, otherwise returns the small repository commit hash.
100100
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_MAPPED_COMMIT'}]" -i "'Hg'" -i None -i "'small-mon'"
101101
[{"commit": {"Hg": bin("a38079aa278633d9e69eb2d90d393b7fec83b09a")},
102102
"translated": {"Hg": bin("a61c0a2e580a4d7c742858d8ebf1a469a7de0839")}}]
103-
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i None -i "'small-mon'"
104-
[{"commit": {"Hg": bin("e6c6c1f94d10495f8c94d81ae5f125bd89f82b8a")},
105-
"translated": {"Hg": bin("a61c0a2e580a4d7c742858d8ebf1a469a7de0839")}}]
103+
$ hg debugapi -e committranslateids -i "[{'Hg': '$LARGE_REPO_ONLY_COMMIT'}]" -i "'Hg'" -i None -i "'small-mon'" -i "'exact'"
104+
[]
106105

107106
# Translate both the LARGE_REPO_-MAPPED-COMMIT and LARGE-REPO-ONLY-COMMIT from the small repository
108107
# FIXME: Both LARGE_REPO_ONLY_COMMIT and LARGE_REPO-MAPPED-COMMIT incorrectly mapped the same small commit

eden/mononoke/tests/integration/library.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1738,7 +1738,7 @@ function x_repo_lookup() {
17381738
SOURCE_REPO="$1"
17391739
TARGET_REPO="$2"
17401740
HASH="$3"
1741-
TRANSLATED=$(hg debugapi -e committranslateids -i "[{'Hg': '$HASH'}]" -i "'Hg'" -i "'$SOURCE_REPO'" -i "'$TARGET_REPO'")
1741+
TRANSLATED=$(hg debugapi -e committranslateids -i "[{'Hg': '$HASH'}]" -i "'Hg'" -i "'$SOURCE_REPO'" -i "'$TARGET_REPO'" -i "'equivalent'")
17421742
hg debugshell <<EOF
17431743
print(hex(${TRANSLATED}[0]["translated"]["Hg"]))
17441744
EOF

eden/scm/lib/eagerepo/src/api.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,7 @@ impl SaplingRemoteApi for EagerRepo {
10711071
scheme: CommitIdScheme,
10721072
from_repo: Option<String>,
10731073
to_repo: Option<String>,
1074+
lookup_behavior: Option<String>,
10741075
) -> Result<Response<CommitTranslateIdResponse>, SaplingRemoteApiError> {
10751076
debug!("files {commits:?} -> {scheme:?}");
10761077

eden/scm/lib/edenapi/src/client.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,7 @@ impl Client {
10181018
scheme: CommitIdScheme,
10191019
from_repo: Option<String>,
10201020
to_repo: Option<String>,
1021+
lookup_behavior: Option<String>,
10211022
) -> Result<Response<CommitTranslateIdResponse>, SaplingRemoteApiError> {
10221023
tracing::info!(
10231024
"Requesting commit id translation for {} commits into {:?}",
@@ -1036,6 +1037,7 @@ impl Client {
10361037
scheme,
10371038
from_repo: from_repo.clone(),
10381039
to_repo: to_repo.clone(),
1040+
lookup_behavior: lookup_behavior.clone(),
10391041
};
10401042
self.log_request(&req, "commit_translate_id");
10411043
req
@@ -1953,13 +1955,15 @@ impl SaplingRemoteApi for Client {
19531955
scheme: CommitIdScheme,
19541956
from_repo: Option<String>,
19551957
to_repo: Option<String>,
1958+
lookup_behavior: Option<String>,
19561959
) -> Result<Response<CommitTranslateIdResponse>, SaplingRemoteApiError> {
19571960
self.with_retry(|this| {
19581961
this.commit_translate_id_attempt(
19591962
commits.clone(),
19601963
scheme.clone(),
19611964
from_repo.clone(),
19621965
to_repo.clone(),
1966+
lookup_behavior.clone(),
19631967
)
19641968
.boxed()
19651969
})

eden/scm/lib/edenapi/trait/src/api.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -402,8 +402,9 @@ pub trait SaplingRemoteApi: Send + Sync + 'static {
402402
scheme: CommitIdScheme,
403403
from_repo: Option<String>,
404404
to_repo: Option<String>,
405+
lookup_behavior: Option<String>,
405406
) -> Result<Response<CommitTranslateIdResponse>, SaplingRemoteApiError> {
406-
let _ = (commits, scheme, from_repo, to_repo);
407+
let _ = (commits, scheme, from_repo, to_repo, lookup_behavior);
407408
Err(SaplingRemoteApiError::NotSupported)
408409
}
409410

eden/scm/lib/edenapi/types/src/commit.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,8 @@ pub struct CommitTranslateIdRequest {
501501
pub from_repo: Option<String>,
502502
#[id(4)]
503503
pub to_repo: Option<String>,
504+
#[id(5)]
505+
pub lookup_behavior: Option<String>,
504506
}
505507

506508
#[auto_wire]

eden/scm/saplingnative/bindings/modules/pyedenapi/src/client.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -568,9 +568,10 @@ py_class!(pub class client |py| {
568568
scheme: Serde<CommitIdScheme>,
569569
fromrepo: Option<String> = None,
570570
torepo: Option<String> = None,
571+
lookup_behavior: Option<String> = None,
571572
) -> PyResult<TStream<anyhow::Result<Serde<CommitTranslateIdResponse>>>> {
572573
let api = self.inner(py).as_ref();
573-
let responses = py.allow_threads(|| block_unless_interrupted(api.commit_translate_id(commits.0, scheme.0, fromrepo, torepo)))
574+
let responses = py.allow_threads(|| block_unless_interrupted(api.commit_translate_id(commits.0, scheme.0, fromrepo, torepo, lookup_behavior)))
574575
.map_pyerr(py)?
575576
.map_pyerr(py)?;
576577
Ok(responses.entries.map_ok(Serde).map_err(Into::into).into())

0 commit comments

Comments
 (0)