Re: [PATCH v7 25/31] prune: strategies for linked checkouts

2014-07-19 Thread Duy Nguyen
On Sat, Jul 19, 2014 at 1:17 AM, Thomas Rast t...@thomasrast.ch wrote:
 I get this from t2026.2 under valgrind:

   ==21334== Conditional jump or move depends on uninitialised value(s)
   ==21334==at 0x46D49B: prune_repos_dir (prune.c:182)
   ==21334==by 0x46D8C0: cmd_prune (prune.c:252)
   ==21334==by 0x405C2F: run_builtin (git.c:351)
   ==21334==by 0x405E47: handle_builtin (git.c:530)
   ==21334==by 0x405F6B: run_argv (git.c:576)
   ==21334==by 0x40610B: main (git.c:663)
   ==21334==  Uninitialised value was created by a stack allocation
   ==21334==at 0x46D3BB: prune_repos_dir (prune.c:169)

Will fix. Thanks.
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 25/31] prune: strategies for linked checkouts

2014-07-18 Thread Thomas Rast
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:

 (alias R=$GIT_COMMON_DIR/repos/id)

  - linked checkouts are supposed to keep its location in $R/gitdir up
to date. The use case is auto fixup after a manual checkout move.

  - linked checkouts are supposed to update mtime of $R/gitdir. If
$R/gitdir's mtime is older than a limit, and it points to nowhere,
repos/id is to be pruned.

  - If $R/locked exists, repos/id is not supposed to be pruned. If
$R/locked exists and $R/gitdir's mtime is older than a really long
limit, warn about old unused repo.

  - git checkout --to is supposed to make a hard link named $R/link
pointing to the .git file on supported file systems to help detect
the user manually deleting the checkout. If $R/link exists and its
link count is greated than 1, the repo is kept.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  Documentation/git-prune.txt|  3 +
  Documentation/gitrepository-layout.txt | 19 ++
  builtin/checkout.c | 14 +
  builtin/prune.c| 99 
 ++
  setup.c| 13 
  t/t2026-prune-linked-checkouts.sh (new +x) | 84 +

I get this from t2026.2 under valgrind:

  ==21334== Conditional jump or move depends on uninitialised value(s)
  ==21334==at 0x46D49B: prune_repos_dir (prune.c:182)
  ==21334==by 0x46D8C0: cmd_prune (prune.c:252)
  ==21334==by 0x405C2F: run_builtin (git.c:351)
  ==21334==by 0x405E47: handle_builtin (git.c:530)
  ==21334==by 0x405F6B: run_argv (git.c:576)
  ==21334==by 0x40610B: main (git.c:663)
  ==21334==  Uninitialised value was created by a stack allocation
  ==21334==at 0x46D3BB: prune_repos_dir (prune.c:169)
  ==21334== 
  {
 insert_a_suppression_name_here
 Memcheck:Cond
 fun:prune_repos_dir
 fun:cmd_prune
 fun:run_builtin
 fun:handle_builtin
 fun:run_argv
 fun:main
  }
  not ok 2 - prune files inside $GIT_DIR/repos
  #
  #   mkdir .git/repos 
  #   : .git/repos/abc 
  #   git prune --repos --verbose actual 
  #   cat expect EOF 
  #   Removing repos/abc: not a valid directory
  #   EOF
  #   test_i18ncmp expect actual 
  #   ! test -f .git/repos/abc 
  #   ! test -d .git/repos
  #

I think it's because of the early 'return 0' ...

 +static int prune_repo_dir(const char *id, struct stat *st, struct strbuf 
 *reason)
 +{
 + char *path;
 + int fd, len;
 +
 + if (!is_directory(git_path(repos/%s, id))) {
 + strbuf_addf(reason, _(Removing repos/%s: not a valid 
 directory), id);
 + return 1;
 + }
 + if (file_exists(git_path(repos/%s/locked, id)))
 + return 0;

in this line, before the stat() actually runs, which then in the
condition ...

 + if (stat(git_path(repos/%s/gitdir, id), st)) {
 + st-st_mtime = expire;
 + strbuf_addf(reason, _(Removing repos/%s: gitdir file does not 
 exist), id);
 + return 1;
 + }
[...]
 +}
 +
 +static void prune_repos_dir(void)
 +{
[...]
 + struct stat st;
[...]
 + if (!prune_repo_dir(d-d_name, st, reason) ||
 + st.st_mtime  expire)

causes the second arm to be evaluated when st.st_mtime is not
initialized yet.  Can you look into this?

-- 
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 25/31] prune: strategies for linked checkouts

2014-07-12 Thread Nguyễn Thái Ngọc Duy
(alias R=$GIT_COMMON_DIR/repos/id)

 - linked checkouts are supposed to keep its location in $R/gitdir up
   to date. The use case is auto fixup after a manual checkout move.

 - linked checkouts are supposed to update mtime of $R/gitdir. If
   $R/gitdir's mtime is older than a limit, and it points to nowhere,
   repos/id is to be pruned.

 - If $R/locked exists, repos/id is not supposed to be pruned. If
   $R/locked exists and $R/gitdir's mtime is older than a really long
   limit, warn about old unused repo.

 - git checkout --to is supposed to make a hard link named $R/link
   pointing to the .git file on supported file systems to help detect
   the user manually deleting the checkout. If $R/link exists and its
   link count is greated than 1, the repo is kept.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-prune.txt|  3 +
 Documentation/gitrepository-layout.txt | 19 ++
 builtin/checkout.c | 14 +
 builtin/prune.c| 99 ++
 setup.c| 13 
 t/t2026-prune-linked-checkouts.sh (new +x) | 84 +
 6 files changed, 232 insertions(+)
 create mode 100755 t/t2026-prune-linked-checkouts.sh

diff --git a/Documentation/git-prune.txt b/Documentation/git-prune.txt
index 7a493c8..50e39ec 100644
--- a/Documentation/git-prune.txt
+++ b/Documentation/git-prune.txt
@@ -48,6 +48,9 @@ OPTIONS
 --expire time::
Only expire loose objects older than time.
 
+--repos::
+   Prune directories in $GIT_DIR/repos.
+
 head...::
In addition to objects
reachable from any of our references, keep objects
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 543d874..bed4f1a 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -256,6 +256,25 @@ repos::
$GIT_COMMON_DIR is set and $GIT_COMMON_DIR/repos will be
used instead.
 
+repos/id/gitdir::
+   A text file containing the absolute path back to the .git file
+   that points to here. This is used to check if the linked
+   repository has been manually removed and there is no need to
+   keep this directory any more. mtime of this file should be
+   updated every time the linked repository is accessed.
+
+repos/id/locked::
+   If this file exists, the linked repository may be on a
+   portable device and not available. It does not mean that the
+   linked repository is gone and `repos/id` could be
+   removed. The file's content contains a reason string on why
+   the repository is locked.
+
+repos/id/link::
+   If this file exists, it is a hard link to the linked .git
+   file. It is used to detect if the linked repository is
+   manually removed.
+
 SEE ALSO
 
 linkgit:git-init[1],
diff --git a/builtin/checkout.c b/builtin/checkout.c
index fe24766..5b93f49 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -898,12 +898,22 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
junk_git_dir = sb_repo.buf;
is_junk = 1;
 
+   /*
+* lock the incomplete repo so prune won't delete it, unlock
+* after the preparation is over.
+*/
+   strbuf_addf(sb, %s/locked, sb_repo.buf);
+   write_file(sb.buf, 1, initializing\n);
+
strbuf_addf(sb_git, %s/.git, path);
if (safe_create_leading_directories_const(sb_git.buf))
die_errno(_(could not create leading directories of '%s'),
  sb_git.buf);
junk_work_tree = path;
 
+   strbuf_reset(sb);
+   strbuf_addf(sb, %s/gitdir, sb_repo.buf);
+   write_file(sb.buf, 1, %s\n, real_path(sb_git.buf));
write_file(sb_git.buf, 1, gitdir: %s/repos/%s\n,
   real_path(get_git_common_dir()), name);
/*
@@ -912,6 +922,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
 * value would do because this value will be ignored and
 * replaced at the next (real) checkout.
 */
+   strbuf_reset(sb);
strbuf_addf(sb, %s/HEAD, sb_repo.buf);
write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1));
strbuf_reset(sb);
@@ -930,6 +941,9 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
ret = run_command(cp);
if (!ret)
is_junk = 0;
+   strbuf_reset(sb);
+   strbuf_addf(sb, %s/locked, sb_repo.buf);
+   unlink_or_warn(sb.buf);
strbuf_release(sb);
strbuf_release(sb_repo);
strbuf_release(sb_git);
diff --git a/builtin/prune.c b/builtin/prune.c
index 144a3bd..28b7adf 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -112,6 +112,95 @@ static void prune_object_dir(const char *path)
}
 }
 
+static int prune_repo_dir(const char *id, struct stat *st, struct