Re: [PATCH 1/3] t5004: test ZIP archives with many entries
On Sat, Aug 22, 2015 at 3:06 PM, René Scharfe wrote:
> A ZIP file directory has a 16-bit field for the number of entries it
> contains. There are 64-bit extensions to deal with that. Demonstrate
> that git archive --format=zip currently doesn't use them and instead
> overflows the field.
>
> InfoZIP's unzip doesn't care about this field and extracts all files
> anyway. Software that uses the directory for presenting a filesystem
> like view quickly -- notably Windows -- depends on it, but doesn't
> lend itself to an automatic test case easily. Use InfoZIP's zipinfo,
> which probably isn't available everywhere but at least can provides
> *some* way to check this field.
>
> To speed things up a bit create and commit only a subset of the files
> and build a fake tree out of duplicates and pass that to git archive.
>
> Signed-off-by: Rene Scharfe
> ---
> diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
> index 654adda..c6bd729 100755
> --- a/t/t5004-archive-corner-cases.sh
> +++ b/t/t5004-archive-corner-cases.sh
> @@ -115,4 +115,44 @@ test_expect_success 'archive empty subtree by direct
> pathspec' '
> check_dir extract sub
> '
>
> +ZIPINFO=zipinfo
> +
> +test_lazy_prereq ZIPINFO '
> + n=$("$ZIPINFO" "$TEST_DIRECTORY"/t5004/empty.zip | sed -n "2s/.* //p")
> + test "x$n" = "x0"
> +'
Unfortunately, this sed expression isn't portable due to dissimilar
output of various zipinfo implementations. On Linux, the output of
zipinfo is:
$ zipinfo t/t5004/empty.zip
Archive: t/t5004/empty.zip
Zip file size: 62 bytes, number of entries: 0
Empty zipfile.
$
however, on Mac OS X:
$ zipinfo t/t5004/empty.zip
Archive: t/t5004/empty.zip 62 bytes 0 files
Empty zipfile.
$
and on FreeBSD, the zipinfo command seems to have been removed
altogether in favor of "unzip -Z" (emulate zipinfo).
One might hope that "unzip -Z" would be a reasonable replacement for
zipinfo, however, it is apparently only partially implemented on
FreeBSD, and requires that -1 be passed, as well. Even with "unzip -Z
-1", there are issues. The output on Linux and Mac OS X is:
$ unzip -Z -1 t/t5004/empty.zip
Empty zipfile.
$
but FreeBSD differs:
$ unzip -Z -1 t/t5004/empty.zip
$
With a non-empty zip file, the output is identical on all platforms:
$ unzip -Z -1 twofiles.zip
file1
file2
$
So, if you combine that with "wc -l" or test_line_count, you may have
a portable and reliable entry counter.
More below...
> +test_expect_failure ZIPINFO 'zip archive with many entries' '
> + # add a directory with 256 files
> + mkdir 00 &&
> + for a in 0 1 2 3 4 5 6 7 8 9 a b c d e f
> + do
> + for b in 0 1 2 3 4 5 6 7 8 9 a b c d e f
> + do
> + : >00/$a$b
> + done
> + done &&
> + git add 00 &&
> + git commit -m "256 files in 1 directory" &&
> +
> + # duplicate it to get 65536 files in 256 directories
> + subtree=$(git write-tree --prefix=00/) &&
> + for c in 0 1 2 3 4 5 6 7 8 9 a b c d e f
> + do
> + for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f
> + do
> + echo "04 tree $subtree $c$d"
> + done
> + done >tree &&
> + tree=$(git mktree +
> + # zip them
> + git archive -o many.zip $tree &&
> +
> + # check the number of entries in the ZIP file directory
> + expr 65536 + 256 >expect &&
> + "$ZIPINFO" many.zip | head -2 | sed -n "2s/.* //p" >actual &&
With these three patches applied, Mac OS X has trouble with 'many.zip':
$ unzip -Z -1 many.zip
warning [many.zip]: 76 extra bytes at beginning or within zipfile
(attempting to process anyway)
error [many.zip]: reported length of central directory is
-76 bytes too long (Atari STZip zipfile? J.H.Holm ZIPSPLIT 1.1
zipfile?). Compensating...
00/
00/00
...
ff/ff
error: expected central file header signature not found (file
#65793). (please check that you have transferred or created the
zipfile in the appropriate BINARY mode and that you have compiled
UnZip properly)
And FreeBSD doesn't like it either:
$ unzip -Z -1 many.zip
unzip: Invalid central directory signature
$
> + test_cmp expect actual
> +'
> +
> test_done
> --
> 2.5.0
--
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] am: terminate state files with a newline
On Thu, Aug 20, 2015 at 11:40:20AM -0700, Junio C Hamano wrote:
> SZEDER Gábor writes:
>
> > The format of the files '.git/rebase-apply/{next,last}' changed slightly
> > with the recent builtin 'git am' conversion: while these files were
> > newline-terminated when written by the scripted version, the ones written
> > by the builtin are not.
>
> Thanks for noticing; that should be corrected, I think.
Okay then, this patch should correct this.
Did we ever explictly allow external programs to poke around the
contents of the .git/rebase-apply directory? I think it may not be so
good, as it means that it may not be possible to switch the storage
format in the future (e.g. to allow atomic modifications, maybe?) :-/ .
Regards,
Paul
-- >8 --
Subject: [PATCH] am: terminate state files with a newline
Since builtin/am.c replaced git-am.sh in 783d7e8 (builtin-am: remove
redirection to git-am.sh, 2015-08-04), the state files written by git-am
did not terminate with a newline.
This is because the code in builtin/am.c did not write the newline to
the state files.
While the git codebase has no problems with the missing newline,
external software which read the contents of the state directory may be
strict about the existence of the terminating newline, and would thus
break.
Fix this by correcting the relevant calls to write_file() to ensure that
the state files written terminate with a newline, matching how git-am.sh
behaves.
While we are fixing the write_file() calls, fix the writing of the
"dirtyindex" file as well -- we should be creating an empty file to
match the behavior of git-am.sh.
Reported-by: SZEDER Gábor
Signed-off-by: Paul Tan
---
builtin/am.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 1399c8d..2e57fad 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -994,13 +994,13 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
if (state->rebasing)
state->threeway = 1;
- write_file(am_path(state, "threeway"), 1, state->threeway ? "t" : "f");
+ write_file(am_path(state, "threeway"), 1, "%s\n", state->threeway ? "t"
: "f");
- write_file(am_path(state, "quiet"), 1, state->quiet ? "t" : "f");
+ write_file(am_path(state, "quiet"), 1, "%s\n", state->quiet ? "t" :
"f");
- write_file(am_path(state, "sign"), 1, state->signoff ? "t" : "f");
+ write_file(am_path(state, "sign"), 1, "%s\n", state->signoff ? "t" :
"f");
- write_file(am_path(state, "utf8"), 1, state->utf8 ? "t" : "f");
+ write_file(am_path(state, "utf8"), 1, "%s\n", state->utf8 ? "t" : "f");
switch (state->keep) {
case KEEP_FALSE:
@@ -1016,9 +1016,9 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
die("BUG: invalid value for state->keep");
}
- write_file(am_path(state, "keep"), 1, "%s", str);
+ write_file(am_path(state, "keep"), 1, "%s\n", str);
- write_file(am_path(state, "messageid"), 1, state->message_id ? "t" :
"f");
+ write_file(am_path(state, "messageid"), 1, "%s\n", state->message_id ?
"t" : "f");
switch (state->scissors) {
case SCISSORS_UNSET:
@@ -1034,10 +1034,10 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
die("BUG: invalid value for state->scissors");
}
- write_file(am_path(state, "scissors"), 1, "%s", str);
+ write_file(am_path(state, "scissors"), 1, "%s\n", str);
sq_quote_argv(&sb, state->git_apply_opts.argv, 0);
- write_file(am_path(state, "apply-opt"), 1, "%s", sb.buf);
+ write_file(am_path(state, "apply-opt"), 1, "%s\n", sb.buf);
if (state->rebasing)
write_file(am_path(state, "rebasing"), 1, "%s", "");
@@ -1045,7 +1045,7 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
write_file(am_path(state, "applying"), 1, "%s", "");
if (!get_sha1("HEAD", curr_head)) {
- write_file(am_path(state, "abort-safety"), 1, "%s",
sha1_to_hex(curr_head));
+ write_file(am_path(state, "abort-safety"), 1, "%s\n",
sha1_to_hex(curr_head));
if (!state->rebasing)
update_ref("am", "ORIG_HEAD", curr_head, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
@@ -1060,9 +1060,9 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
* session is in progress, they should be written last.
*/
- write_file(am_path(state, "next"), 1, "%d", state->cur);
+ write_file(am_path(state, "next"), 1, "%d\n", state->cur);
- write_file(am_path(state, "last"), 1, "%d", state->last);
+ write_file(am_path(state, "last"), 1, "%d\n", state->last);
strbuf_release(&sb);
}
@@ -1095,12 +1095,12 @@ static void am_next(struct am_
Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems
Lars - thanks for persisting with this!
I'm still trying to fully understand what's going on here - can you
point out where I've got it wrong below please!
The server is on Linux, and is case-sensitive. For whatever reason
(probably people committing changes on Windows in the first place)
we've ended up with a P4 repo that has differences in path case that
need to be ignored, with all upper-case paths being mapped to
lower-case.
e.g. the *real* depot might be:
//depot/path/File1
//depot/PATH/File2
git-p4 clone should return this case-folded on Windows (and Linux?)
$GIT_DIR/path/File1
$GIT_DIR/path/File2
(Am I right in thinking that this change folds the directory, but not
the filename?)
I don't really understand why a dictionary is required to do this
though - is there a reason we can't just lowercase all incoming paths?
Which is what the existing code in p4StartWith() is trying to do. That
code lowercases the *entire* path including the filename; this change
seems to leave the filename portion unchanged. I guess though that the
answer may be to do with your finding that P4 makes up the case of
directories based on lexicographic ordering - is that the basic
problem?
For a large repo, this approach is surely going to be really slow.
Additionally, could you update your patch to add some words to
Documentation/git-p4.txt please?
A few other comments inline.
Thanks!
Luke
On 21 August 2015 at 18:19, wrote:
> From: Lars Schneider
>
> PROBLEM:
> We run P4 servers on Linux and P4 clients on Windows. For an unknown
> reason the file path for a number of files in P4 does not match the
> directory path with respect to case sensitivity.
>
> E.g. `p4 files` might return
> //depot/path/to/file1
> //depot/PATH/to/file2
>
> If you use P4/P4V then these files end up in the same directory, e.g.
> //depot/path/to/file1
> //depot/path/to/file2
>
> If you use git-p4 then all files not matching the correct file path
> (e.g. `file2`) will be ignored.
I'd like to think that the existing code that checks core.ignorecase
should handle this. I haven't tried it myself though.
>
> SOLUTION:
> Identify path names that are different with respect to case sensitivity.
> If there are any then run `p4 dirs` to build up a dictionary
> containing the "correct" cases for each path. It looks like P4
> interprets "correct" here as the existing path of the first file in a
> directory. The path dictionary is used later on to fix all paths.
>
> This is only applied if the parameter "--ignore-path-case" is passed to
> the git-p4 clone command.
>
> Signed-off-by: Lars Schneider
> ---
> git-p4.py | 82 ++--
> t/t9821-git-p4-path-variations.sh | 109
> ++
> 2 files changed, 187 insertions(+), 4 deletions(-)
> create mode 100755 t/t9821-git-p4-path-variations.sh
>
> diff --git a/git-p4.py b/git-p4.py
> index 073f87b..61a587b 100755
> --- a/git-p4.py
> +++ b/git-p4.py
> @@ -1931,7 +1931,7 @@ class View(object):
> (self.client_prefix, clientFile))
> return clientFile[len(self.client_prefix):]
>
> -def update_client_spec_path_cache(self, files):
> +def update_client_spec_path_cache(self, files, fixPathCase = None):
> """ Caching file paths by "p4 where" batch query """
>
> # List depot file paths exclude that already cached
> @@ -1950,6 +1950,8 @@ class View(object):
> if "unmap" in res:
> # it will list all of them, but only one not unmap-ped
> continue
> +if fixPathCase:
> +res['depotFile'] = fixPathCase(res['depotFile'])
> self.client_spec_path_cache[res['depotFile']] =
> self.convert_client_path(res["clientFile"])
>
> # not found files or unmap files set to ""
> @@ -1987,6 +1989,7 @@ class P4Sync(Command, P4UserMap):
> help="Maximum number of changes to
> import"),
> optparse.make_option("--changes-block-size",
> dest="changes_block_size", type="int",
> help="Internal block size to use when
> iteratively calling p4 changes"),
> +optparse.make_option("--ignore-path-case",
> dest="ignorePathCase", action="store_true"),
> optparse.make_option("--keep-path", dest="keepRepoPath",
> action='store_true',
> help="Keep entire BRANCH/DIR/SUBDIR
> prefix during import"),
> optparse.make_option("--use-client-spec",
> dest="useClientSpec", action='store_true',
> @@ -2017,6 +2020,7 @@ class P4Sync(Command, P4UserMap):
> self.maxChanges = ""
> self.changes_block_size = None
> self.keepRepoPath = False
> +self.ignorePathCase = False
> self.depotPaths = None
> self.p4BranchesInGit = []
> self.cloneExclude = []
> @@ -2049,7 +2053,8 @@ class P4Sync(Command, P4UserMap):
>
[PATCH v5 1/2] worktree: add 'for_each_worktree' function
for_each_worktree iterates through each worktree and invokes a callback
function. The main worktree (if not bare) is always encountered first,
followed by worktrees created by `git worktree add`.
If the callback function returns a non-zero value, iteration stops, and
the callback's return value is returned from the for_each_worktree
function.
Signed-off-by: Michael Rappazzo
---
builtin/worktree.c | 83 ++
1 file changed, 83 insertions(+)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 430b51e..7b3cb96 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -26,6 +26,14 @@ static int show_only;
static int verbose;
static unsigned long expire;
+/*
+ * The signature for the callback function for the for_each_worktree()
+ * function below. The memory pointed to by the callback arguments
+ * is only guaranteed to be valid for the duration of a single
+ * callback invocation.
+ */
+typedef int each_worktree_fn(const char *path, const char *git_dir, void
*cb_data);
+
static int prune_worktree(const char *id, struct strbuf *reason)
{
struct stat st;
@@ -359,6 +367,81 @@ static int add(int ac, const char **av, const char *prefix)
return add_worktree(path, branch, &opts);
}
+/*
+ * Iterate through each worktree and invoke the callback function. If
+ * the callback function returns non-zero, the iteration stops, and
+ * this function returns that return value
+ */
+static int for_each_worktree(each_worktree_fn fn, void *cb_data)
+{
+ struct strbuf worktree_path = STRBUF_INIT;
+ struct strbuf worktree_git = STRBUF_INIT;
+ const char *common_dir;
+ int main_is_bare = 0;
+ int ret = 0;
+
+ common_dir = get_git_common_dir();
+ if (!strcmp(common_dir, get_git_dir())) {
+ /* simple case - this is the main repo */
+ main_is_bare = is_bare_repository();
+ if (!main_is_bare) {
+ strbuf_addstr(&worktree_path, get_git_work_tree());
+ } else {
+ strbuf_addstr(&worktree_path, common_dir);
+ }
+ } else {
+ strbuf_addstr(&worktree_path, common_dir);
+ /* If the common_dir DOES NOT end with '/.git', then it is bare
*/
+ main_is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
+ }
+ strbuf_addstr(&worktree_git, worktree_path.buf);
+
+ if (!main_is_bare) {
+ strbuf_addstr(&worktree_git, "/.git");
+
+ ret = fn(worktree_path.buf, worktree_git.buf, cb_data);
+ if (ret)
+ goto done;
+ }
+ strbuf_addstr(&worktree_git, "/worktrees");
+
+ if (is_directory(worktree_git.buf)) {
+ DIR *dir = opendir(worktree_git.buf);
+ if (dir) {
+ struct stat st;
+ struct dirent *d;
+ size_t base_path_len = worktree_git.len;
+
+ while ((d = readdir(dir)) != NULL) {
+ if (!strcmp(d->d_name, ".") ||
!strcmp(d->d_name, ".."))
+ continue;
+
+ strbuf_setlen(&worktree_git, base_path_len);
+ strbuf_addf(&worktree_git, "/%s/gitdir",
d->d_name);
+
+ if (stat(worktree_git.buf, &st)) {
+ fprintf(stderr, "Unable to read
worktree git dir: %s\n", worktree_git.buf);
+ continue;
+ }
+
+ strbuf_reset(&worktree_path);
+ strbuf_read_file(&worktree_path,
worktree_git.buf, st.st_size);
+ strbuf_strip_suffix(&worktree_path, "/.git\n");
+
+ strbuf_strip_suffix(&worktree_git, "/gitdir");
+ ret = fn(worktree_path.buf, worktree_git.buf,
cb_data);
+ if (ret)
+ break;
+ }
+ }
+ closedir(dir);
+ }
+done:
+ strbuf_release(&worktree_git);
+ strbuf_release(&worktree_path);
+ return ret;
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
--
2.5.0
--
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 v5 0/2] Worktree: for-each function and list command
Changes since v4: - reduced memory usage by reusing string buffer variables - re-scoped variables in the for-each function - added tests for 'worktree list' with bare repos Notes from previous discussion: - The following snippet: > + /* If the common_dir DOES NOT end with '/.git', then it is bare */ > + main_is_bare = !strbuf_strip_suffix(&worktree_path, "/.git"); This code is run when the current dir is in a linked worktree (not the primary worktree). In that context, the git config _does_ report that 'core.bare' is true when the primary repo is bare, however `is_bare_repository()` returns false. The worktree_path also needs to the "/.git" removed from it. Therefore, I opted to keep the code like this. Michael Rappazzo (2): worktree: add 'for_each_worktree' function worktree: add 'list' command Documentation/git-worktree.txt | 11 +++- builtin/worktree.c | 138 + t/t2027-worktree-list.sh | 81 3 files changed, 229 insertions(+), 1 deletion(-) create mode 100755 t/t2027-worktree-list.sh -- 2.5.0 -- 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 v5 2/2] worktree: add 'list' command
'git worktree list' uses the for_each_worktree function to iterate,
and outputs in the format: ' ()'
Signed-off-by: Michael Rappazzo
---
Documentation/git-worktree.txt | 11 +-
builtin/worktree.c | 55
t/t2027-worktree-list.sh | 81 ++
3 files changed, 146 insertions(+), 1 deletion(-)
create mode 100755 t/t2027-worktree-list.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index fb68156..e953b4e 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git worktree add' [-f] [--detach] [-b ] []
'git worktree prune' [-n] [-v] [--expire ]
+'git worktree list' [--path-only]
DESCRIPTION
---
@@ -59,6 +60,12 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+list::
+
+List the main worktree followed by all of the linked worktrees. The default
+format of the list includes the full path to the worktree and the branch or
+revision that the head of that worktree is currently pointing to.
+
OPTIONS
---
@@ -93,6 +100,9 @@ OPTIONS
--expire ::
With `prune`, only expire unused working trees older than .
+--path-only
+ With `list`, only show the worktree path
+
DETAILS
---
Each linked working tree has a private sub-directory in the repository's
@@ -167,7 +177,6 @@ performed manually, such as:
- `remove` to remove a linked working tree and its administrative files (and
warn if the working tree is dirty)
- `mv` to move or rename a working tree and update its administrative files
-- `list` to list linked working trees
- `lock` to prevent automatic pruning of administrative files (for instance,
for a working tree on a portable device)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b3cb96..673f292 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,7 @@
static const char * const worktree_usage[] = {
N_("git worktree add [] "),
N_("git worktree prune []"),
+ N_("git worktree list []"),
NULL
};
@@ -442,6 +443,58 @@ done:
return ret;
}
+/* list callback data */
+struct list_opts {
+ int path_only;
+};
+
+static int print_worktree_details(const char *path, const char *git_dir, void
*cb_data)
+{
+ struct strbuf head_file = STRBUF_INIT;
+ struct strbuf head_ref = STRBUF_INIT;
+ struct stat st;
+ struct list_opts *opts = cb_data;
+ const char *ref_prefix = "ref: refs/heads/";
+
+ strbuf_addf(&head_file, "%s/HEAD", git_dir);
+ if (!opts->path_only && !stat(head_file.buf, &st)) {
+ strbuf_read_file(&head_ref, head_file.buf, st.st_size);
+ strbuf_strip_suffix(&head_ref, "\n");
+
+ if (starts_with(head_ref.buf, ref_prefix)) {
+ /* branch checked out */
+ strbuf_remove(&head_ref, 0, strlen(ref_prefix));
+ /* } else {
+* headless -- no-op
+*/
+ }
+ printf("%s (%s)\n", path, head_ref.buf);
+ } else {
+ printf("%s\n", path);
+ }
+
+ strbuf_release(&head_ref);
+ strbuf_release(&head_file);
+ return 0;
+}
+
+static int list(int ac, const char **av, const char *prefix)
+{
+ struct list_opts opts;
+ struct option options[] = {
+ OPT_BOOL(0, "path-only", &opts.path_only, N_("only show the
path of the worktree")),
+ OPT_END()
+ };
+
+ opts.path_only = 0;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac)
+ usage_with_options(worktree_usage, options);
+
+ return for_each_worktree(&print_worktree_details, &opts);
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -454,5 +507,7 @@ int cmd_worktree(int ac, const char **av, const char
*prefix)
return add(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "prune"))
return prune(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "list"))
+ return list(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
new file mode 100755
index 000..66a635a
--- /dev/null
+++ b/t/t2027-worktree-list.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+test_description='test git worktree list'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit init
+'
+
+test_expect_success '"list" all worktrees from main' '
+ echo "$(git rev-parse --show-toplevel) ($(git symbolic-ref --short
HEAD))" >expect &&
+ git worktree add --detach here master &&
+ echo "$(git -C here rev-parse --show-toplevel) ($(git -C here
rev-parse HEAD))" >>expect &&
+ git worktree list >actual &&
+ test_cmp expect actual &&
[PATCH 1/3] t5004: test ZIP archives with many entries
A ZIP file directory has a 16-bit field for the number of entries it
contains. There are 64-bit extensions to deal with that. Demonstrate
that git archive --format=zip currently doesn't use them and instead
overflows the field.
InfoZIP's unzip doesn't care about this field and extracts all files
anyway. Software that uses the directory for presenting a filesystem
like view quickly -- notably Windows -- depends on it, but doesn't
lend itself to an automatic test case easily. Use InfoZIP's zipinfo,
which probably isn't available everywhere but at least can provides
*some* way to check this field.
To speed things up a bit create and commit only a subset of the files
and build a fake tree out of duplicates and pass that to git archive.
Signed-off-by: Rene Scharfe
---
t/t5004-archive-corner-cases.sh | 40
1 file changed, 40 insertions(+)
diff --git a/t/t5004-archive-corner-cases.sh b/t/t5004-archive-corner-cases.sh
index 654adda..c6bd729 100755
--- a/t/t5004-archive-corner-cases.sh
+++ b/t/t5004-archive-corner-cases.sh
@@ -115,4 +115,44 @@ test_expect_success 'archive empty subtree by direct
pathspec' '
check_dir extract sub
'
+ZIPINFO=zipinfo
+
+test_lazy_prereq ZIPINFO '
+ n=$("$ZIPINFO" "$TEST_DIRECTORY"/t5004/empty.zip | sed -n "2s/.* //p")
+ test "x$n" = "x0"
+'
+
+test_expect_failure ZIPINFO 'zip archive with many entries' '
+ # add a directory with 256 files
+ mkdir 00 &&
+ for a in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+ do
+ for b in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+ do
+ : >00/$a$b
+ done
+ done &&
+ git add 00 &&
+ git commit -m "256 files in 1 directory" &&
+
+ # duplicate it to get 65536 files in 256 directories
+ subtree=$(git write-tree --prefix=00/) &&
+ for c in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+ do
+ for d in 0 1 2 3 4 5 6 7 8 9 a b c d e f
+ do
+ echo "04 tree $subtree $c$d"
+ done
+ done >tree &&
+ tree=$(git mktree expect &&
+ "$ZIPINFO" many.zip | head -2 | sed -n "2s/.* //p" >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.5.0
--
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 3/3] archive-zip: support more than 65535 entries
Support more than 65535 entries cleanly by writing a "zip64 end of
central directory record" (with a 64-bit field for the number of
entries) before the usual "end of central directory record" (which
contains only a 16-bit field). InfoZIP's zip does the same.
Archives with 65535 or less entries are not affected.
Programs that extract all files like InfoZIP's zip and 7-Zip
ignored the field and could extract all files already. Software
that relies on the ZIP file directory to show a list of contained
files quickly to simulate to normal directory like Windows'
built-in ZIP functionality only saw a subset of the included files.
Windows supports ZIP64 since Vista according to
https://en.wikipedia.org/wiki/Zip_%28file_format%29#ZIP64.
Suggested-by: Johannes Schauer
Signed-off-by: Rene Scharfe
---
archive-zip.c | 93 +++--
t/t5004-archive-corner-cases.sh | 2 +-
2 files changed, 91 insertions(+), 4 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index 2a76156..9db4735 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -16,7 +16,9 @@ static unsigned int zip_dir_size;
static unsigned int zip_offset;
static unsigned int zip_dir_offset;
-static unsigned int zip_dir_entries;
+static uint64_t zip_dir_entries;
+
+static unsigned int max_creator_version;
#define ZIP_DIRECTORY_MIN_SIZE (1024 * 1024)
#define ZIP_STREAM (1 << 3)
@@ -86,6 +88,28 @@ struct zip_extra_mtime {
unsigned char _end[1];
};
+struct zip64_dir_trailer {
+ unsigned char magic[4];
+ unsigned char record_size[8];
+ unsigned char creator_version[2];
+ unsigned char version[2];
+ unsigned char disk[4];
+ unsigned char directory_start_disk[4];
+ unsigned char entries_on_this_disk[8];
+ unsigned char entries[8];
+ unsigned char size[8];
+ unsigned char offset[8];
+ unsigned char _end[1];
+};
+
+struct zip64_dir_trailer_locator {
+ unsigned char magic[4];
+ unsigned char disk[4];
+ unsigned char offset[8];
+ unsigned char number_of_disks[4];
+ unsigned char _end[1];
+};
+
/*
* On ARM, padding is added at the end of the struct, so a simple
* sizeof(struct ...) reports two bytes more than the payload size
@@ -98,6 +122,12 @@ struct zip_extra_mtime {
#define ZIP_EXTRA_MTIME_SIZE offsetof(struct zip_extra_mtime, _end)
#define ZIP_EXTRA_MTIME_PAYLOAD_SIZE \
(ZIP_EXTRA_MTIME_SIZE - offsetof(struct zip_extra_mtime, flags))
+#define ZIP64_DIR_TRAILER_SIZE offsetof(struct zip64_dir_trailer, _end)
+#define ZIP64_DIR_TRAILER_RECORD_SIZE \
+ (ZIP64_DIR_TRAILER_SIZE - \
+offsetof(struct zip64_dir_trailer, creator_version))
+#define ZIP64_DIR_TRAILER_LOCATOR_SIZE \
+ offsetof(struct zip64_dir_trailer_locator, _end)
static void copy_le16(unsigned char *dest, unsigned int n)
{
@@ -113,6 +143,31 @@ static void copy_le32(unsigned char *dest, unsigned int n)
dest[3] = 0xff & (n >> 030);
}
+static void copy_le64(unsigned char *dest, uint64_t n)
+{
+ dest[0] = 0xff & n;
+ dest[1] = 0xff & (n >> 010);
+ dest[2] = 0xff & (n >> 020);
+ dest[3] = 0xff & (n >> 030);
+ dest[4] = 0xff & (n >> 040);
+ dest[5] = 0xff & (n >> 050);
+ dest[6] = 0xff & (n >> 060);
+ dest[7] = 0xff & (n >> 070);
+}
+
+static uint64_t clamp_max(uint64_t n, uint64_t max, int *clamped)
+{
+ if (n <= max)
+ return n;
+ *clamped = 1;
+ return max;
+}
+
+static void copy_le16_clamp(unsigned char *dest, uint64_t n, int *clamped)
+{
+ copy_le16(dest, clamp_max(n, 0x, clamped));
+}
+
static void *zlib_deflate_raw(void *data, unsigned long size,
int compression_level,
unsigned long *compressed_size)
@@ -282,6 +337,9 @@ static int write_zip_entry(struct archiver_args *args,
sha1_to_hex(sha1));
}
+ if (creator_version > max_creator_version)
+ max_creator_version = creator_version;
+
if (buffer && method == 8) {
out = deflated = zlib_deflate_raw(buffer, size,
args->compression_level,
@@ -439,20 +497,49 @@ static int write_zip_entry(struct archiver_args *args,
return 0;
}
+static void write_zip64_trailer(void)
+{
+ struct zip64_dir_trailer trailer64;
+ struct zip64_dir_trailer_locator locator64;
+
+ copy_le32(trailer64.magic, 0x06064b50);
+ copy_le64(trailer64.record_size, ZIP64_DIR_TRAILER_RECORD_SIZE);
+ copy_le16(trailer64.creator_version, max_creator_version);
+ copy_le16(trailer64.version, 45);
+ copy_le32(trailer64.disk, 0);
+ copy_le32(trailer64.directory_start_disk, 0);
+ copy_le64(trailer64.entries_on_this_disk, zip_dir_entries);
+ copy_le64(trailer64.entries, zip_dir_entries);
+ copy_le64(trailer64.size, zip_dir_offset
[PATCH 2/3] archive-zip: use a local variable to store the creator version
Use a simpler conditional right next to the code which makes a higher creator version necessary -- namely symlink handling and support for executable files -- instead of a long line with a ternary operator. The resulting code has more lines but is simpler and allows reuse of the value easily. Signed-off-by: Rene Scharfe --- archive-zip.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/archive-zip.c b/archive-zip.c index ae3d67f..2a76156 100644 --- a/archive-zip.c +++ b/archive-zip.c @@ -223,6 +223,7 @@ static int write_zip_entry(struct archiver_args *args, unsigned long size; int is_binary = -1; const char *path_without_prefix = path + args->baselen; + unsigned int creator_version = 0; crc = crc32(0, NULL, 0); @@ -251,6 +252,8 @@ static int write_zip_entry(struct archiver_args *args, method = 0; attr2 = S_ISLNK(mode) ? ((mode | 0777) << 16) : (mode & 0111) ? ((mode) << 16) : 0; + if (S_ISLNK(mode) || (mode & 0111)) + creator_version = 0x0317; if (S_ISREG(mode) && args->compression_level != 0 && size > 0) method = 8; @@ -303,8 +306,7 @@ static int write_zip_entry(struct archiver_args *args, } copy_le32(dirent.magic, 0x02014b50); - copy_le16(dirent.creator_version, - S_ISLNK(mode) || (S_ISREG(mode) && (mode & 0111)) ? 0x0317 : 0); + copy_le16(dirent.creator_version, creator_version); copy_le16(dirent.version, 10); copy_le16(dirent.flags, flags); copy_le16(dirent.compression_method, method); -- 2.5.0 -- 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 v3 7/8] branch.c: use 'ref-filter' APIs
On Sat, Aug 22, 2015 at 9:21 PM, Christian Couder wrote: > On Sat, Aug 22, 2015 at 8:51 AM, Karthik Nayak wrote: >> >> The test t1430 'git branch shows badly named ref' has been changed to >> check the stderr for the warning regarding the broken ref. This is >> done as ref-filter throws a warning for broken refs rather than >> directly printing them. > > [...] > >> diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh >> index 16d0b8b..db3627e 100755 >> --- a/t/t1430-bad-ref-name.sh >> +++ b/t/t1430-bad-ref-name.sh >> @@ -41,7 +41,7 @@ test_expect_success 'fast-import: fail on invalid branch >> name "bad[branch]name"' >> test_expect_success 'git branch shows badly named ref' ' >> cp .git/refs/heads/master .git/refs/heads/broken...ref && >> test_when_finished "rm -f .git/refs/heads/broken...ref" && >> - git branch >output && >> + git branch 2>output && >> grep -e "broken\.\.\.ref" output >> ' > > Maybe the test could be renamed to 'git branch warns about badly named > ref' and maybe you could also check that "broken\.\.\.ref" is not > printed on stdout. > The name change sounds reasonable, do we really need to check for it in the stdout? -- Regards, Karthik Nayak -- 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] Fix `git rev-list --show-notes HEAD` when there are no notes
On Sat, Aug 22, 2015 at 5:14 PM, Johannes Schindelin wrote: > The `format_display_notes()` function clearly assumes that the data > structure holding the notes has been initialized already, i.e. that the > `display_notes_trees` variable is no longer `NULL`. > > However, when there are no notes whatsoever, this variable is still > `NULL`, even after initialization. > > So let's be graceful and just return if that data structure is `NULL`. > > Reported in https://github.com/msysgit/git/issues/363. > > Signed-off-by: Johannes Schindelin Acked-by: Johan Herland -- Johan Herland, www.herland.net -- 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 v3 7/8] branch.c: use 'ref-filter' APIs
On Sat, Aug 22, 2015 at 8:51 AM, Karthik Nayak wrote: > > The test t1430 'git branch shows badly named ref' has been changed to > check the stderr for the warning regarding the broken ref. This is > done as ref-filter throws a warning for broken refs rather than > directly printing them. [...] > diff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh > index 16d0b8b..db3627e 100755 > --- a/t/t1430-bad-ref-name.sh > +++ b/t/t1430-bad-ref-name.sh > @@ -41,7 +41,7 @@ test_expect_success 'fast-import: fail on invalid branch > name "bad[branch]name"' > test_expect_success 'git branch shows badly named ref' ' > cp .git/refs/heads/master .git/refs/heads/broken...ref && > test_when_finished "rm -f .git/refs/heads/broken...ref" && > - git branch >output && > + git branch 2>output && > grep -e "broken\.\.\.ref" output > ' Maybe the test could be renamed to 'git branch warns about badly named ref' and maybe you could also check that "broken\.\.\.ref" is not printed on stdout. Thanks, Christian. -- 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] Fix `git rev-list --show-notes HEAD` when there are no notes
The `format_display_notes()` function clearly assumes that the data
structure holding the notes has been initialized already, i.e. that the
`display_notes_trees` variable is no longer `NULL`.
However, when there are no notes whatsoever, this variable is still
`NULL`, even after initialization.
So let's be graceful and just return if that data structure is `NULL`.
Reported in https://github.com/msysgit/git/issues/363.
Signed-off-by: Johannes Schindelin
---
notes.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/notes.c b/notes.c
index df08209..24a335a 100644
--- a/notes.c
+++ b/notes.c
@@ -1266,7 +1266,10 @@ void format_display_notes(const unsigned char
*object_sha1,
struct strbuf *sb, const char *output_encoding, int
raw)
{
int i;
- assert(display_notes_trees);
+
+ if (!display_notes_trees)
+ return;
+
for (i = 0; display_notes_trees[i]; i++)
format_note(display_notes_trees[i], object_sha1, sb,
output_encoding, raw);
--
2.3.1.windows.1.9.g8c01ab4
--
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: Which branch(es) contain certain commits? (was Re: (unknown))
On Sat, Aug 22, 2015 at 2:49 PM, Ivan Chernyavsky wrote > >> Note that "branch --list", "tag --list" and "for-each-ref" are being >> revamped to share more internal code. If you want to pursue this, >> you probably would want to build on top of that effort once it is >> done. That way, you may get "tag --grep=FIX123" for free. >> > > This is interesting. So can I have a look on some repo/branch or just wait > until it'll be merged? > This should be what you're looking for: http://thread.gmane.org/gmane.comp.version-control.git/276363 http://thread.gmane.org/gmane.comp.version-control.git/276377 This is the current progress, the rest is in git/next. There is still a series of porting the printing part of branch.c to be done. -- Regards, Karthik Nayak -- 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: Which branch(es) contain certain commits? (was Re: (unknown))
22.08.2015, 12:19, "Ivan Chernyavsky" :
> One thing I'm worried about is that git-branch already has option --all. So
> we'll get a semantics conflict with setup_revisions() ("all branches" vs "all
> refs"). This will have to be treated carefully, e.g. retrace and fix effects
> of --all after setup_revisions() to include just branches but not other refs.
> Will such mangling be ok? Or could I prepare the call of setup_revisions() in
> a way that only branches will be included by --all? Anyway the semantics of
> --all will be different for git-branch and git-rev-list, but I believe more
> important is to keep it unchanged for git-branch.
>
Please disregard this part. I should just gobble --all with the existing
parse_options() and do not provide it to setup_revisions() altogether.
--
Ivan
--
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: Which branch(es) contain certain commits? (was Re: (unknown))
22.08.2015, 01:39, "Junio C Hamano" :
> Ivan Chernyavsky writes:
>
>> Another problem is that builtin/branch.c currently does not use
>> setup_revisions(), so I'll have to hook it there as well.
>
> Heh, you say "problem" above, but I do not think it is a "problem"
> per-se. If you want to teach branch some preprocessing based on the
> revision traversal API, you naturally need to call setup_revisions().
I meant "my problem" mostly ;) Yes I see git-branch currently has it's own way
of doing things, I assume it avoids more heavy machinery from git-rev-list and
friends.
> The outlined steps above all feel sensible; one thing you did not
> mention is that you may probably have to clear object flags after
> you are done with the initial "--grep" revision traversal, as some
> features of branch may want to use the object flags (e.g. --merged
> would use in_merge_bases()). Other than that, all of it sounds
> easily implementable.
I still need to understand a lot how it all works. Any hint on where to look is
appreciated. (I started from looking on builtin/shortlog.c just because it's
one of the shortest ones using setup_revisions().)
One thing I'm worried about is that git-branch already has option --all. So
we'll get a semantics conflict with setup_revisions() ("all branches" vs "all
refs"). This will have to be treated carefully, e.g. retrace and fix effects of
--all after setup_revisions() to include just branches but not other refs. Will
such mangling be ok? Or could I prepare the call of setup_revisions() in a way
that only branches will be included by --all? Anyway the semantics of --all
will be different for git-branch and git-rev-list, but I believe more important
is to keep it unchanged for git-branch.
> Note that "branch --list", "tag --list" and "for-each-ref" are being
> revamped to share more internal code. If you want to pursue this,
> you probably would want to build on top of that effort once it is
> done. That way, you may get "tag --grep=FIX123" for free.
>
This is interesting. So can I have a look on some repo/branch or just wait
until it'll be merged?
>> That said, do you think the goal is worth such changes?
>
> That is a dangerous question. As Duy already said,
>
>> Probably because nobody is interested and steps up to do it. The lack
>> of response to you mail is a sign.
>
> apparently not many people thought it is worth; otherwise we would
> already have such a feature.
>
> If you are asking me personally, I'm sorry but I have to say no.
>
> The reason why I personally do not think your "branch --grep=FIX123"
> would be very useful to me is because I would imagine that I would
> be interested in learning the exact commit that mentions FIX123 as
> well as which branches contain it, if I had a need for such a
> feature.
>
> That is, it would inherently be two step process for me anyway, i.e.
>
> (1) I'd run "log -p --grep" to find which commits are about FIX123
> and check that what they did indeed make sense; and
>
> (2) I'd then run "branch --contains" to learn which ones are
> already up to date with respect to the fix.
>
> Your "branch --grep=FIX123" that only tells me the names of branches
> would have no use in that workflow, as it would not even give me an
> indication that the request --grep=FIX123 found the right commit in
> the first place.
Yes, I see your point. But let me also explain my use case so it'll be probably
more clear. Sorry if it's too long.
I work for HP as SCM and build manager for a product >15 years old. One of this
product's greatest strenghts always was support for many old releases and
customizations for particular users. So right now we are providing support for
10 subsequent releases (oldest one released back in 2006), and each release has
5 to 10 "customer" branches with their specific enhancements and fixes.
(Though, as you would expect, all the fixes are generally merged to mainline,
some customers are reluctant to apply fixes they don't need, so even here we
must be flexible when preparing a patch.) So basically now I'm managing around
60 public branches in the repository.
SCM was ClearCase at the beginning, then StarTeam (whoa, complete disaster). At
this point I joined the team and performed migration to Git last year (took a
whole year). So now our history is a mixture of ancient baselines from
ClearCase and artificial branches imported from StarTeam (thanks to my
colleague who wrote a Git helper for StarTeam,
https://github.com/AndreyPavlenko/git-st). So many changes which are 100%
identical are represented by different commits, only identifiable with
--cherry-pick option to git-log and friends. Even if I spent a great amount of
time manually cutting and glueing history in Git to have the shortest logs.
And obviously, our workflow is based on cherry-picks, because very very often
fixes and enhancements have to be merged *back* to many old customer branche
GSOC: Unify the commands `git tag -l`, `git brach -l` and `git for-each-ref`
Hello All,
As part of GSoC I'm working on the Unification of 'for-each-ref', 'tag -l'
and 'branch -l'.
Current Progress:
1. Building ref-filter.{c,h} from for-each-ref.
This is the process of creating an initial library for the unification
by moving most of the code from for-each-ref to ref-filter.{c,h}.
Merged into next
2. Add options to ref-filter.
This includes the porting of --points-at, --contains, --merged,
--no-merged options from builtin/branch.c and builtin/tag.c, Also the
implementation of these options into for-each-ref.
Merged to next
3. Port builtin/tag.c to use ref-filter.
Here we port tag.c to use ref-filter and also port the --format,
--sort and --merged and --no-merged options to builtin/tag.c.
Version 13 can be found here:
* http://thread.gmane.org/gmane.comp.version-control.git/276363
There was a lot of discussion on v12 (especially how %(align)...%(end) should be
handling '--quote') which can be found here:
* http://thread.gmane.org/gmane.comp.version-control.git/276133
4. Port builtin/branch.c to use ref-filter filtering APIs.
Here we make use of the filtering APIs provided by ref-filter to filter out
branches. Also make it learn the '--points-at' command.
version 3 can be found here:
* http://thread.gmane.org/gmane.comp.version-control.git/276377
As my mentors suggested, I'm working on the printing aspects of branch.c now.
This involves supporting %(if)%(then)%(else)%(end). and
making branch.c
use show_ref_array_item() rather than using its own implementation.
As the ground has been laid for %(align)%(end) atom, the work shouldn't be
of much difficulty.
Will push most of my work to github: github.com/KarthikNayak/git
--
Regards,
Karthik Nayak
--
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
