Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check path is case conflict when checkout files #2748

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

feiniks
Copy link
Contributor

@feiniks feiniks commented Mar 14, 2024

No description provided.

if (is_path_case_conflict (worktree, path)) {
return;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename 和 delete 应该在 seaf_repo_fetch_and_checkout 里面处理,而不是在现在这个层次更深的函数里面处理。另外,删除文件、重命名文件也是需要检查 case conflict 的。

lib/utils.c Outdated

#ifdef __APPLE__
static gboolean
case_conflict_recursive (const char *worktree, const char *path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看了一下,在导出一个文件的之前,实际上是在 build_checkout_path 这个函数里面创建它的各级父目录的。也就是说,如果它的某一级父目录存在 case conflict,会导致这一级父目录下面多创建出一些空目录来。比如,现有 A/b/c/d 这样的路径,然后现在导出一个 a/e/f/g.txt 文件,导出的结果就会是 A 目录下面多出来一个 e 目录。

现在 build_checkout_path 这个函数里面其实还遗留着一些以前处理 case conflict 代码的痕迹,也就是以前是会在里面处理一下的。只不过以前的逻辑是把名字后面加一个 (case conflict),后来发现效果不好就去掉了。

并且,你要测试确保不导出 case conflict 的文件目录,不会导致它在服务器上被删除。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

现在在调用 build_checkout_path之前都会检查 is_path_case_conflict,所以是不会创建出多余的目录的。

@@ -4661,6 +4661,7 @@ typedef struct FileTxTask {
int result;
gboolean no_checkout;
gboolean force_conflict;
gboolean skip_checkout;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么要增加一个 skip_checkout 字段呢?原来的 no_checkout 就可以了吧。

// Since file creation is asynchronous, the file may not have been created locally at the time of checking for case conflicts,
// so an additional check for the name of the file being created is required.
static gboolean
is_adding_files_case_conflict (GHashTable *adding_files, const char *name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 adding_files 如果只是需要遍历,应该用一个链表更合适。

@@ -4799,13 +4817,15 @@ schedule_file_fetch (GThreadPool *tpool,
DiffEntry *de,
GHashTable *pending_tasks,
GHashTable *conflict_hash,
GHashTable *no_conflict_hash)
GHashTable *no_conflict_hash,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看了一下,conflict_hash 和 no_conflict_hash 两个参数应该是没有用的,可以在整个文件里面清理一下去掉。

现在代码里面还遗留了 case_conflict 这个局部变量,也可以去掉了。

@@ -5069,6 +5099,11 @@ handle_dir_added_de (const char *repo_id,
goto update_index;
}

if (is_path_case_conflict(worktree, de->name)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

由于各种原因没有导出的文件、文件夹,都是要创建 ce 的。否则 index 就会和 fs tree 不一致,可能导致服务器误删文件。

客户端重启之后,扫描 worktree 的时候,判断 worktree 文件是否被删除,会检查 ce->ctime 是否为 0。那些主动不导出的 ce->ctime 都是 0。


if (g_hash_table_size (pending_tasks) == 0)
break;
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里也是,需要创建 ce。实际上 checkout_file_http 一开始就会检查 task->no_checkout,直接返回。这里应该不需要改了。

@@ -5977,7 +6030,9 @@ seaf_repo_fetch_and_checkout (HttpTxTask *http_task, const char *remote_head_id)
ce->ce_mtime.sec, NULL);
}
#else
delete_path (worktree, de->name, de->mode, ce->ce_mtime.sec);
if (!is_path_case_conflict (worktree, de->name)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux 上面不用检查吧。

@@ -6000,7 +6055,9 @@ seaf_repo_fetch_and_checkout (HttpTxTask *http_task, const char *remote_head_id)
continue;
}

delete_worktree_dir (repo_id, http_task->repo_name, &istate, worktree, de->name);
if (!is_path_case_conflict (worktree, de->name)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

各种情况下 case conflict 没有导出改动,都应该保存错误、发送通知、并打印日志。

有一种情况需要特殊处理,就是导出一个新建的文件夹,下面可能有很多文件,现在我们的代码是会转换为单个文件逐一导出,这样可能产生大量的 case conflict 报错。一种处理方案是在 seaf_repo_fetch_and_checkout() 将新建目录转换为单个文件之前,判断一下目录是否 case conflict。

remove_from_index_with_prefix (&istate, de->name, NULL);
} else if (new_path_conflict) {
remove_from_index_with_prefix (&istate, de->new_name, NULL);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename 的情况比较复杂,分为4种情况:

  1. 如果 old 和 new 都没有 conflict,那么正常处理
  2. 如果 old conflict 而 new 不 conflict,那么应该把 old 对应的文件导出出来。这个类似于 ignore on checkout 的处理。
  3. 如果 old 不 conflict 而 new conflict,那么应该把 old 删掉(需要注意检查是否有本地改动)
  4. 如果 old 和 new 都 conflict,那么只需要更新 index 即可。

1、2、3 都需要更新 index。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants