Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems
snip While I was working on the examples for this email reply I realized that the problem is only present for paths given in a client-spec. I added a test case to prove that. That means I don’t need to request *all* paths. I *think* it is sufficient to request the paths mentioned in the client spec which would usually be really fast. Stay tuned for PATCH v5. I've just tried a small experiment with stock unaltered git-p4. - Started p4d with -C1 option to case-fold. - Add some files with different cases of directory (Path/File1, PATH/File2, pATH/File3). - git-p4 clone. The result of the clone is separate directories if I do nothing special (PATH/File1, Path/File2, etc). But if I set core.ignorecase, I get a single case-folded directory, Path/File1, Path/File2, etc. I'm still failing to get how that isn't what you need (other than being a bit obscure to get to the right invocation). I've put a script that shows this here: https://github.com/luked99/quick-git-p4-case-folding-test As mentioned I realized that the problem occurs only if you use client specs. Can you take a look at this test case / run it? https://github.com/larsxschneider/git/blob/d3a191cb5fb4d8f5f48ca9314c772169d5dbf65b/t/t9821-git-p4-path-variations.sh#L112-L127 Does this make sense to you? If you want to I could also modify “quick-git-p4-case-folding-test” to show the problem. Cheers, Lars -- 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 v4] git-p4: fix faulty paths for case insensitive systems
On 24 August 2015 at 13:43, Lars Schneider larsxschnei...@gmail.com wrote: https://github.com/luked99/quick-git-p4-case-folding-test As mentioned I realized that the problem occurs only if you use client specs. Can you take a look at this test case / run it? https://github.com/larsxschneider/git/blob/d3a191cb5fb4d8f5f48ca9314c772169d5dbf65b/t/t9821-git-p4-path-variations.sh#L112-L127 Does this make sense to you? If you want to I could also modify “quick-git-p4-case-folding-test” to show the problem. If you're able to fix my hacky test to show the problem that would be very kind. If the problem only shows up when using client specs, is it possible that the core.ignorecase logic is just missing a code path somewhere? Glancing through the code, stripRepoPath() could perhaps be the culprit? If self.useClientSpec is FALSE, it will do the core.ignorecase trick by calling p4PathStartsWith, but if it is TRUE, it won't. Thanks! Luke -- 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 v4] git-p4: fix faulty paths for case insensitive systems
On 24 August 2015 at 10:51, Lars Schneider larsxschnei...@gmail.com wrote: I'm still trying to fully understand what's going on here - can you point out where I've got it wrong below please! Your welcome + sure :) snip While I was working on the examples for this email reply I realized that the problem is only present for paths given in a client-spec. I added a test case to prove that. That means I don’t need to request *all* paths. I *think* it is sufficient to request the paths mentioned in the client spec which would usually be really fast. Stay tuned for PATCH v5. I've just tried a small experiment with stock unaltered git-p4. - Started p4d with -C1 option to case-fold. - Add some files with different cases of directory (Path/File1, PATH/File2, pATH/File3). - git-p4 clone. The result of the clone is separate directories if I do nothing special (PATH/File1, Path/File2, etc). But if I set core.ignorecase, I get a single case-folded directory, Path/File1, Path/File2, etc. I'm still failing to get how that isn't what you need (other than being a bit obscure to get to the right invocation). I've put a script that shows this here: https://github.com/luked99/quick-git-p4-case-folding-test Thanks! Luke -- 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
Hi Peff, On 2015-08-24 16:43, Jeff King wrote: On Mon, Aug 24, 2015 at 12:23:52PM +0200, Johannes Schindelin wrote: You're right. I think the best approach for now is to error out when `--show-notes` is passed to rev-list. Do you agree? Yes (I imagine you didn't yet read my follow-up patch when you wrote this). :) Yep, and I forgot to reply when I saw it. Thanks for continuing on that front! Ciao, Dscho -- 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 v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 1:30 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). That'd be easy to implement, but I didn't because `git tag -l` is human readable and I didn't see the necessity for having something like `--quote_type` here. Agreed: tag does not have --shell, --python so and does not need it. But that's not my point: you write if used with '--quote', and the option name is not --quote. It should be if used with `--shell`, `--python`, ... then everything -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 Mon, Aug 24, 2015 at 12:23:52PM +0200, Johannes Schindelin wrote: You're right. I think the best approach for now is to error out when `--show-notes` is passed to rev-list. Do you agree? Yes (I imagine you didn't yet read my follow-up patch when you wrote this). :) -Peff -- 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 v13 00/12] port tag.c to use ref-filter APIs
On Mon, Aug 24, 2015 at 1:30 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). That'd be easy to implement, but I didn't because `git tag -l` is human readable and I didn't see the necessity for having something like `--quote_type` here. It'd be better to just use `git for-each-ref refs/tags/` for that. I had already commented on a preliminary version of this series off-list. I think all my previous comments have been taken into account. Thanks, Thanks for the review :) -- 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 v13 11/12] tag.c: implement '--format' option
On Mon, Aug 24, 2015 at 8:44 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 1:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -13,7 +13,8 @@ SYNOPSIS tagname [commit | object] 'git tag' -d tagname... 'git tag' [-n[num]] -l [--contains commit] [--points-at object] - [--column[=options] | --no-column] [--create-reflog] [--sort=key] [pattern...] + [--column[=options] | --no-column] [--create-reflog] [--sort=key] + [--format=format] [pattern...] 'git tag' -v tagname... DESCRIPTION @@ -158,6 +159,11 @@ This option is only applicable when listing tags without annotation lines. The object that the new tag will refer to, usually a commit. Defaults to HEAD. +format:: Shouldn't this be --format format, not just format? We usually use the named argument in the SYNOPSIS for positional arguments, but not for arguments following an option. This is how it was in for-each-ref Documentation, hence to keep it similar I just put format. It's wrong in another place sounds like an argument to fix the other place, not to get it wrong here too ;-). Of course! That was just me justifying my action. I agree, it should be corrected both places. -- 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 v13 00/12] port tag.c to use ref-filter APIs
On Mon, Aug 24, 2015 at 8:46 PM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 1:30 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). That'd be easy to implement, but I didn't because `git tag -l` is human readable and I didn't see the necessity for having something like `--quote_type` here. Agreed: tag does not have --shell, --python so and does not need it. But that's not my point: you write if used with '--quote', and the option name is not --quote. It should be if used with `--shell`, `--python`, ... then everything Oops! misread what you sent, maybe' --quote_type' instead. -- 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 v13 11/12] tag.c: implement '--format' option
On Mon, Aug 24, 2015 at 1:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -13,7 +13,8 @@ SYNOPSIS tagname [commit | object] 'git tag' -d tagname... 'git tag' [-n[num]] -l [--contains commit] [--points-at object] - [--column[=options] | --no-column] [--create-reflog] [--sort=key] [pattern...] + [--column[=options] | --no-column] [--create-reflog] [--sort=key] + [--format=format] [pattern...] 'git tag' -v tagname... DESCRIPTION @@ -158,6 +159,11 @@ This option is only applicable when listing tags without annotation lines. The object that the new tag will refer to, usually a commit. Defaults to HEAD. +format:: Shouldn't this be --format format, not just format? We usually use the named argument in the SYNOPSIS for positional arguments, but not for arguments following an option. This is how it was in for-each-ref Documentation, hence to keep it similar I just put format. -- 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 v13 11/12] tag.c: implement '--format' option
Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 1:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Karthik Nayak karthik@gmail.com writes: --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -13,7 +13,8 @@ SYNOPSIS tagname [commit | object] 'git tag' -d tagname... 'git tag' [-n[num]] -l [--contains commit] [--points-at object] - [--column[=options] | --no-column] [--create-reflog] [--sort=key] [pattern...] + [--column[=options] | --no-column] [--create-reflog] [--sort=key] + [--format=format] [pattern...] 'git tag' -v tagname... DESCRIPTION @@ -158,6 +159,11 @@ This option is only applicable when listing tags without annotation lines. The object that the new tag will refer to, usually a commit. Defaults to HEAD. +format:: Shouldn't this be --format format, not just format? We usually use the named argument in the SYNOPSIS for positional arguments, but not for arguments following an option. This is how it was in for-each-ref Documentation, hence to keep it similar I just put format. It's wrong in another place sounds like an argument to fix the other place, not to get it wrong here too ;-). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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 v2] describe --contains: default to HEAD when no commit-ish is given
'git describe --contains' doesn't default to HEAD when no commit is
given, and it doesn't produce any output, not even an error:
~/src/git ((v2.5.0))$ ./git describe --contains
~/src/git ((v2.5.0))$ ./git describe --contains HEAD
v2.5.0^0
Unlike other 'git describe' options, the '--contains' code path is
implemented by calling 'name-rev' with a bunch of options plus all the
commit-ishes that were passed to 'git describe'. If no commit-ish was
present, then 'name-rev' got invoked with none, which then leads to the
behavior illustrated above.
Porcelain commands usually default to HEAD when no commit-ish is given,
and 'git describe' already does so in all other cases, so it should do
so with '--contains' as well.
Pass HEAD to 'name-rev' when no commit-ish is given on the command line
to make '--contains' behave consistently with other 'git describe'
options. While at it, use argv_array_pushv() instead of the loop to
pass commit-ishes to 'git name-rev'.
'git describe's short help already indicates that the commit-ish is
optional, but the synopsis in the man page doesn't, so update it
accordingly as well.
Signed-off-by: SZEDER Gábor sze...@ira.uka.de
---
Documentation/git-describe.txt | 4 ++--
builtin/describe.c | 8
t/t6120-describe.sh| 8
3 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e045fc73f8..c8f28c8c86 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -9,7 +9,7 @@ git-describe - Describe a commit using the most recent tag
reachable from it
SYNOPSIS
[verse]
-'git describe' [--all] [--tags] [--contains] [--abbrev=n] commit-ish...
+'git describe' [--all] [--tags] [--contains] [--abbrev=n] [commit-ish...]
'git describe' [--all] [--tags] [--contains] [--abbrev=n] --dirty[=mark]
DESCRIPTION
@@ -27,7 +27,7 @@ see the -a and -s options to linkgit:git-tag[1].
OPTIONS
---
commit-ish...::
- Commit-ish object names to describe.
+ Commit-ish object names to describe. Defaults to HEAD if omitted.
--dirty[=mark]::
Describe the working tree.
diff --git a/builtin/describe.c b/builtin/describe.c
index ce36032b1f..9dadfb58c8 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -443,10 +443,10 @@ int cmd_describe(int argc, const char **argv, const char
*prefix)
if (pattern)
argv_array_pushf(args, --refs=refs/tags/%s,
pattern);
}
- while (*argv) {
- argv_array_push(args, *argv);
- argv++;
- }
+ if (argc)
+ argv_array_pushv(args, argv);
+ else
+ argv_array_push(args, HEAD);
return cmd_name_rev(args.argc, args.argv, prefix);
}
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 57d50156bb..bf52a0a1cc 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -115,6 +115,14 @@ check_describe e-3-* --first-parent --tags
check_describe $(git rev-parse --short HEAD) --exact-match --always HEAD
+test_expect_success 'describe --contains defaults to HEAD without commit-ish' '
+ echo A^0 expect
+ git checkout A
+ test_when_finished git checkout -
+ git describe --contains actual
+ test_cmp expect actual
+'
+
: err.expect
check_describe A --all A^0
test_expect_success 'no warning was displayed for A' '
--
2.5.0.418.gdd37a9b
--
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] describe --contains: default to HEAD when no commit-ish is given
Quoting Junio C Hamano gits...@pobox.com:
@@ -443,10 +443,13 @@ int cmd_describe(int argc, const char **argv,
const char *prefix)
if (pattern)
argv_array_pushf(args, --refs=refs/tags/%s,
pattern);
}
- while (*argv) {
- argv_array_push(args, *argv);
- argv++;
- }
+ if (argc)
What would this code do to 'describe --all --contains'? was my
knee-jerk reaction, but the options are all parsed by this code and
nothing is delegated to name-rev, so 'if (!argc)' here is truly the
lack of any revisions to be described, which is good.
Exactly. parse-opts removes all --options from argv as it processes
them, barfs at --unknown-options, so all what remains must be treated
as a commit-ish. And if nothing is left, well, then there was none
given.
+ while (*argv) {
+ argv_array_push(args, *argv);
+ argv++;
+ }
+ else
+ argv_array_push(args, HEAD);
By the way, I usually prefer a fatter 'else' clause when everything
else is equal, i.e.
if (!argc)
argv_array_push(args, HEAD); /* default to HEAD */
else {
while (*argv) {
...
}
}
because it is easy to miss tiny else-clause while reading code, but
it is harder to miss tiny then-clause. In this case, however, the
while loop can be replaced with argv_array_pushv() these days, so
perhaps
if (!argc)
argv_array_push(args, HEAD); /* default to HEAD ... */
else
argv_array_pushv(args, argv); /* or relay what we got */
or something?
Indeed, I didn't notice argv_array_pushv() being added, log tells me
it happened quite recently. I suppose with both branches becoming a
one-liner the order of them can remain what it was in the patch,
this sparing the negation from 'if (!argc)'.
v2 comes in a minute.
Gábor
--
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
bug: git branch -d and case-insensitive file-systems
I use git (2.2.1) on OS X (10.9.5) and recently my repo got into a bad state. I think this involves a mis-handling of case-insensitive file systems. This reproduces the problem: git init Initialized empty Git repository in /Users/aarond_local/code/git-test/.git/ git commit --allow-empty -m 'first commit' [master (root-commit) 923d8b8] first commit git checkout -b feature Switched to a new branch 'feature' git checkout -b Feature fatal: A branch named 'Feature' already exists. git checkout -B Feature Switched to and reset branch 'Feature' git branch -d feature Deleted branch feature (was 923d8b8). git log fatal: bad default revision 'HEAD' This is the behavior when there isn't a case mismatch, which is what I would have expected in the previous case as well: git init Initialized empty Git repository in /Users/aarond_local/code/git-test/.git/ git commit --allow-empty -m 'first commit' [master (root-commit) 48df19f] first commit git checkout -b feature Switched to a new branch 'feature' git branch -d feature error: Cannot delete the branch 'feature' which you are currently on. I can also reproduce the issue on git 2.5.0. -Aaron Dufour -- 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: Submodule, subtree, or something else?
On Sun, Aug 23, 2015 at 7:11 AM, Jānis Rukšāns janis.ruks...@gmail.com wrote: On Pk, 2015-08-21 at 17:07 -0700, Stefan Beller wrote: On Fri, Aug 21, 2015 at 3:47 PM, Jānis Rukšāns janis.ruks...@gmail.com wrote: A major drawback of submodules in my opinion is the inability to make a full clone from an existing one without having access to the central repository, which is something I have to do from time to time. Can you elaborate on that a bit more? git clone --recurse-submodules should do that no matter which remote you contact? I mean that if I have cloned a repository with submodules, cloning that repository with --recurse-submodules will either access the central server if absolute URLs are used, or requires additional clones for each submodule. For example git clone --recursive http://somewhere/projectA.git git clone --recursive file://$(pwd)/projectA projectA.tmp The second command will cause the submodules to be downloaded again, or expect them to be found in $(pwd). IIUC, the second command will lookup the submodules in $(pwd), but if they are not there they are skipped, so all of the existing submodules are cloned. Why do you need more submodules in the tmp clone than in $(pwd)/projectA would be my next question. But I see your point now. Or am I mistaken, or doing something wrong? -- 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 5/5] write_file(): clean up transitional mess of flag words and terminating LF
Because the function adds necessary LF at the end of an incomplete
line for all callers that do not pass the WRITE_FILE_BINARY option,
and no caller of the function calls with that option, stop callers
to add LF at the end of the payload they pass to the function.
Also, change the callers that pass 1 to flags, that is now a no-op,
to pass 0. In order to catch stray callers (and possible topics in
flight) that still pass 1 to ask the function to die upon error,
protect it with an assert().
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 4 ++--
builtin/init-db.c | 2 +-
builtin/worktree.c | 10 +-
daemon.c | 2 +-
setup.c| 2 +-
submodule.c| 2 +-
transport.c| 2 +-
wrapper.c | 7 ++-
8 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 3423aa3..d804b12 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -199,13 +199,13 @@ static inline const char *am_path(const struct am_state
*state, const char *path
static int write_state_text(const struct am_state *state,
const char *name, const char *string)
{
- return write_file(am_path(state, name), 1, %s\n, string);
+ return write_file(am_path(state, name), 0, %s, string);
}
static int write_state_count(const struct am_state *state,
const char *name, int value)
{
- return write_file(am_path(state, name), 1, %d\n, value);
+ return write_file(am_path(state, name), 0, %d, value);
}
static int write_state_bool(const struct am_state *state,
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 49df78d..84d27b1 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -378,7 +378,7 @@ static void separate_git_dir(const char *git_dir)
die_errno(_(unable to move %s to %s), src, git_dir);
}
- write_file(git_link, 1, gitdir: %s\n, git_dir);
+ write_file(git_link, 0, gitdir: %s, git_dir);
}
int init_db(const char *template_dir, unsigned int flags)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a264ee..cc0981f 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -213,7 +213,7 @@ static int add_worktree(const char *path, const char
**child_argv)
* after the preparation is over.
*/
strbuf_addf(sb, %s/locked, sb_repo.buf);
- write_file(sb.buf, 1, initializing\n);
+ write_file(sb.buf, 0, initializing);
strbuf_addf(sb_git, %s/.git, path);
if (safe_create_leading_directories_const(sb_git.buf))
@@ -223,8 +223,8 @@ static int add_worktree(const char *path, const char
**child_argv)
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/worktrees/%s\n,
+ write_file(sb.buf, 0, %s, real_path(sb_git.buf));
+ write_file(sb_git.buf, 0, gitdir: %s/worktrees/%s,
real_path(get_git_common_dir()), name);
/*
* This is to keep resolve_ref() happy. We need a valid HEAD
@@ -241,10 +241,10 @@ static int add_worktree(const char *path, const char
**child_argv)
die(_(unable to resolve HEAD));
strbuf_reset(sb);
strbuf_addf(sb, %s/HEAD, sb_repo.buf);
- write_file(sb.buf, 1, %s\n, sha1_to_hex(rev));
+ write_file(sb.buf, 0, %s, sha1_to_hex(rev));
strbuf_reset(sb);
strbuf_addf(sb, %s/commondir, sb_repo.buf);
- write_file(sb.buf, 1, ../..\n);
+ write_file(sb.buf, 0, ../..);
fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name);
diff --git a/daemon.c b/daemon.c
index d3d3e43..30a3fb4 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1376,7 +1376,7 @@ int main(int argc, char **argv)
sanitize_stdfds();
if (pid_file)
- write_file(pid_file, 1, %PRIuMAX\n, (uintmax_t) getpid());
+ write_file(pid_file, 0, %PRIuMAX, (uintmax_t) getpid());
/* prepare argv for serving-processes */
cld_argv = xmalloc(sizeof (char *) * (argc + 2));
diff --git a/setup.c b/setup.c
index 718f4e1..49675eb 100644
--- a/setup.c
+++ b/setup.c
@@ -404,7 +404,7 @@ static void update_linked_gitdir(const char *gitfile, const
char *gitdir)
strbuf_addf(path, %s/gitfile, gitdir);
if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL))
- write_file(path.buf, WRITE_FILE_GENTLY, %s\n, gitfile);
+ write_file(path.buf, WRITE_FILE_GENTLY, %s, gitfile);
strbuf_release(path);
}
diff --git a/submodule.c b/submodule.c
index 700bbf4..c22fd04 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1103,7 +1103,7 @@ void connect_work_tree_and_git_dir(const char *work_tree,
const char *git_dir)
/* Update gitfile */
strbuf_addf(file_name, %s/.git, work_tree);
- write_file(file_name.buf, 1, gitdir: %s\n,
+
[PATCH 4/5] write_file(): do not leave incomplete line at the end
All existing callers to this function use it to produce a text file
or an empty file, and a new callsite that mimick them must end their
payload with a LF. If they forget to do so, the resulting file will
end with an incomplete line.
Introduce WRITE_FILE_BINARY flag bit, which no existing callers pass,
and unless that bit is set, make sure that write_file() adds an extra
LF at the end of an incomplete line as necessary.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
cache.h | 1 +
wrapper.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/cache.h b/cache.h
index f105235..dbfa4fa 100644
--- a/cache.h
+++ b/cache.h
@@ -1552,6 +1552,7 @@ static inline ssize_t write_str_in_full(int fd, const
char *str)
*/
#define WRITE_FILE_UNUSED_0 (10)
#define WRITE_FILE_GENTLY (11)
+#define WRITE_FILE_BINARY (12)
__attribute__((format (printf, 3, 4)))
extern int write_file(const char *path, unsigned flags, const char *fmt, ...);
diff --git a/wrapper.c b/wrapper.c
index 68d45b6..4cd2ca3 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -635,6 +635,9 @@ int write_file(const char *path, unsigned flags, const char
*fmt, ...)
va_start(params, fmt);
strbuf_vaddf(sb, fmt, params);
va_end(params);
+ if (!(flags WRITE_FILE_BINARY))
+ strbuf_complete_line(sb);
+
if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
int err = errno;
close(fd);
--
2.5.0-568-g53a3e28
--
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 v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak karthik@gmail.com writes: Interdiff: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. That sounds like a buggy behaviour that we may want to fix later, though. Perhaps document it as a known bug, e.g. Currently it does not work well when used with language-specific quoting like --shell, etc. (while punting on fixing the implementation for now)? -- 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
Karthik Nayak karthik@gmail.com writes: 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 2output 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? I think Christian meant we should check in both, i.e. git branch output 2error grep broken\.\.\.ref error ! grep broken\.\.\.ref output or something like that. That way, you are effectively specifying in the test that badly named refs must not be included in the output, so somebody who changes that in the future needs to justify why including them in the output is a good idea when updating the test. -- 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: Submodule, subtree, or something else?
On P , 2015-08-24 at 09:51 -0700, Stefan Beller wrote: IIUC, the second command will lookup the submodules in $(pwd), but if they are not there they are skipped, so all of the existing submodules are cloned. Why do you need more submodules in the tmp clone than in $(pwd)/projectA would be my next question. But I see your point now. The $(pwd) was just an example to illustrate my point. The actual use case is that I would be hacking on something at work, notice that it is already late and I have to catch the last bus home, yet I don't want to postpone whatever I was working on until the next day. So I would do git commit -a -m [WIP] Stuff, finish at home to save my work so far, go home, and clone / fetch it over ssh. Another important factor is that a lot of our code can be meaningfully tested only on the actual hardware, and is built in a VM. Quite often getting things right involve many iterations of hack hack hack, git commit --amend, fetch reset --hard in the VM, build, test, repeat. Being able to clone / fetch directly from the copy I am working on makes it a lot easier. As I wrote in the other e-mail, I managed to achieve the desired result by using ./submodule (without .git suffix) as the submodule URL, and creating a file named submodule in the bare repo with 'gitdir: ../submodule.git' as it's contents, but I'm not sure whether it is a good idea or not. Jānis -- 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 2/5] builtin/am: make sure state files are text
We forgot to terminate the payload given to write_file() with LF,
resulting in files that ends with an incomplete line.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 4d34dc5..3423aa3 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -199,13 +199,13 @@ static inline const char *am_path(const struct am_state
*state, const char *path
static int write_state_text(const struct am_state *state,
const char *name, const char *string)
{
- return write_file(am_path(state, name), 1, %s, string);
+ return write_file(am_path(state, name), 1, %s\n, string);
}
static int write_state_count(const struct am_state *state,
const char *name, int value)
{
- return write_file(am_path(state, name), 1, %d, value);
+ return write_file(am_path(state, name), 1, %d\n, value);
}
static int write_state_bool(const struct am_state *state,
--
2.5.0-568-g53a3e28
--
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 0/5] am state file fix with write_file() clean-up
So here is an solution based on the write_file() is primarily to produce text, so it should be able to correct the incomplete line at the end approach. The first one is Peff's idea to consolidate callers in am, in a more concrete form. The second is the fix to $gmane/276238. The remainder is to clean up write_file() helper function. All callers except for two were passing 1 as one parameter, whose meaning was not all obvious to a casual reader. In patch 3/5, we flip the default behaviour of write_file() to die upon error unless explicitly asked not to with WRITE_FILE_GENTLY flag, and change the two oddball callers to pass this new flag. In patch 4/5, we enhance the default behaviour of write_file() to complete an incomplete line at the end, unless asked not to with WRITE_FILE_BINARY flag; nobody passes this because all existing callers want to produce a text file. In patch 5/5, the transitional noise left by patches 3 and 4 are cleaned up by updating the non-binary callers not to add LF themselves and by changing the callers that pass 1 as flags parameter to pass 0 (as bit (10) is a no-op since patch 3/5). The series is built on top of b5e8235, the current tip of the pt/am-builtin-options topic. Junio C Hamano (5): builtin/am: introduce write_state_*() helper functions builtin/am: make sure state files are text write_file(): introduce an explicit WRITE_FILE_GENTLY request write_file(): do not leave incomplete line at the end write_file(): clean up transitional mess of flag words and terminating LF builtin/am.c | 68 -- builtin/init-db.c | 2 +- builtin/worktree.c | 10 cache.h| 16 - daemon.c | 2 +- setup.c| 2 +- submodule.c| 2 +- transport.c| 2 +- wrapper.c | 13 +-- 9 files changed, 77 insertions(+), 40 deletions(-) -- 2.5.0-568-g53a3e28 -- 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 1/5] builtin/am: introduce write_state_*() helper functions
There are many calls to write_file() that repeats the same pattern
in the implementation of the builtin version of am, and they all
share the same traits, i.e they
- produce a text file with a single string in it;
- have enough information to produce the entire contents of that
file;
- generate the pathname of the file by making a call to am_path(); and
- they ask write_file() to die() upon failure.
The slight differences among the call sites throw them into roughly
three variants:
- many write either t or f based on a boolean value to a file;
- some write the integer value in decimal text;
- some others write more general string, e.g. an object name in
hex, an empty string (i.e. the presense of the file itself serves
as a flag), etc.
Introduce three helpers, write_state_bool(), write_state_count() and
write_state_text(), to reduce direct calls to write_file().
This is a preparatory step for the next step to ensure that no
state file this command leaves in $GIT_DIR is with an incomplete
line at the end.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 68
1 file changed, 41 insertions(+), 27 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 634f7a7..4d34dc5 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -194,6 +194,27 @@ static inline const char *am_path(const struct am_state
*state, const char *path
}
/**
+ * For convenience to call write_file()
+ */
+static int write_state_text(const struct am_state *state,
+ const char *name, const char *string)
+{
+ return write_file(am_path(state, name), 1, %s, string);
+}
+
+static int write_state_count(const struct am_state *state,
+const char *name, int value)
+{
+ return write_file(am_path(state, name), 1, %d, value);
+}
+
+static int write_state_bool(const struct am_state *state,
+ const char *name, int value)
+{
+ return write_state_text(state, name, value ? t : f);
+}
+
+/**
* If state-quiet is false, calls fprintf(fp, fmt, ...), and appends a newline
* at the end.
*/
@@ -362,7 +383,7 @@ static void write_author_script(const struct am_state
*state)
sq_quote_buf(sb, state-author_date);
strbuf_addch(sb, '\n');
- write_file(am_path(state, author-script), 1, %s, sb.buf);
+ write_state_text(state, author-script, sb.buf);
strbuf_release(sb);
}
@@ -1000,13 +1021,10 @@ 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, quiet), 1, state-quiet ? t : f);
-
- write_file(am_path(state, sign), 1, state-signoff ? t : f);
-
- write_file(am_path(state, utf8), 1, state-utf8 ? t : f);
+ write_state_bool(state, threeway, state-threeway);
+ write_state_bool(state, quiet, state-quiet);
+ write_state_bool(state, sign, state-signoff);
+ write_state_bool(state, utf8, state-utf8);
switch (state-keep) {
case KEEP_FALSE:
@@ -1022,9 +1040,8 @@ 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, messageid), 1, state-message_id ? t :
f);
+ write_state_text(state, keep, str);
+ write_state_bool(state, messageid, state-message_id);
switch (state-scissors) {
case SCISSORS_UNSET:
@@ -1039,24 +1056,23 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
default:
die(BUG: invalid value for state-scissors);
}
-
- write_file(am_path(state, scissors), 1, %s, str);
+ write_state_text(state, scissors, str);
sq_quote_argv(sb, state-git_apply_opts.argv, 0);
- write_file(am_path(state, apply-opt), 1, %s, sb.buf);
+ write_state_text(state, apply-opt, sb.buf);
if (state-rebasing)
- write_file(am_path(state, rebasing), 1, %s, );
+ write_state_text(state, rebasing, );
else
- write_file(am_path(state, applying), 1, %s, );
+ write_state_text(state, applying, );
if (!get_sha1(HEAD, curr_head)) {
- write_file(am_path(state, abort-safety), 1, %s,
sha1_to_hex(curr_head));
+ write_state_text(state, abort-safety, sha1_to_hex(curr_head));
if (!state-rebasing)
update_ref(am, ORIG_HEAD, curr_head, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
} else {
- write_file(am_path(state, abort-safety), 1, %s, );
+ write_state_text(state, abort-safety, );
if (!state-rebasing)
[PATCH 3/5] write_file(): introduce an explicit WRITE_FILE_GENTLY request
All callers except for two ask this function to die upon error by
passing fatal=1; turn the parameter to a more generic unsigned flag
bag of bits, introduce an explicit WRITE_FILE_GENTLY bit and change
these two callers to pass that bit.
This is in preparation to add one more bit to this flag word.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
cache.h | 15 ++-
setup.c | 2 +-
transport.c | 2 +-
wrapper.c | 3 ++-
4 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 6bb7119..f105235 100644
--- a/cache.h
+++ b/cache.h
@@ -1539,8 +1539,21 @@ static inline ssize_t write_str_in_full(int fd, const
char *str)
{
return write_in_full(fd, str, strlen(str));
}
+
+/*
+ * Create a new file by specifying its full contents via fmt and the
+ * remainder of args that are used like 'printf()' args. Die upon
+ * an error unless WRITE_FILE_GENTLY flag is set, in which case return
+ * a negative number to signal an error.
+ *
+ * For historical reasons, the LSB of flags word is set by many
+ * callers to explicitly ask the function to die upon error, but now
+ * it is the default.
+ */
+#define WRITE_FILE_UNUSED_0 (10)
+#define WRITE_FILE_GENTLY (11)
__attribute__((format (printf, 3, 4)))
-extern int write_file(const char *path, int fatal, const char *fmt, ...);
+extern int write_file(const char *path, unsigned flags, const char *fmt, ...);
/* pager.c */
extern void setup_pager(void);
diff --git a/setup.c b/setup.c
index 5f9f07d..718f4e1 100644
--- a/setup.c
+++ b/setup.c
@@ -404,7 +404,7 @@ static void update_linked_gitdir(const char *gitfile, const
char *gitdir)
strbuf_addf(path, %s/gitfile, gitdir);
if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL))
- write_file(path.buf, 0, %s\n, gitfile);
+ write_file(path.buf, WRITE_FILE_GENTLY, %s\n, gitfile);
strbuf_release(path);
}
diff --git a/transport.c b/transport.c
index 40692f8..e1821a4 100644
--- a/transport.c
+++ b/transport.c
@@ -291,7 +291,7 @@ static int write_one_ref(const char *name, const struct
object_id *oid,
strbuf_addstr(buf, name);
if (safe_create_leading_directories(buf-buf) ||
- write_file(buf-buf, 0, %s\n, oid_to_hex(oid)))
+ write_file(buf-buf, WRITE_FILE_GENTLY, %s\n, oid_to_hex(oid)))
return error(problems writing temporary file %s: %s,
buf-buf, strerror(errno));
strbuf_setlen(buf, len);
diff --git a/wrapper.c b/wrapper.c
index e451463..68d45b6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -621,8 +621,9 @@ char *xgetcwd(void)
return strbuf_detach(sb, NULL);
}
-int write_file(const char *path, int fatal, const char *fmt, ...)
+int write_file(const char *path, unsigned flags, const char *fmt, ...)
{
+ int fatal = !(flags WRITE_FILE_GENTLY);
struct strbuf sb = STRBUF_INIT;
va_list params;
int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0666);
--
2.5.0-568-g53a3e28
--
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: Submodule, subtree, or something else?
On Sv, 2015-08-23 at 17:13 -0600, Cox, Michael wrote: You might want to take a look at how the Boost (boost.org) project uses submodules. They use submodules for each library. I know they use relative paths in their .gitmodules file to avoid the problem you're referring to regarding git clone --recurse-submodules. Thanks! I had a look at their setup, and they are using ../libx.git for submodules, which unfortunately breaks when cloning from another working copy: $ git clone --recursive file:///tmp/gittest/repo.a/main.git main.work Cloning into 'main.work'... snip Submodule 'liba' (file:///tmp/gittest/repo.a/liba.git) registered for path 'liba' Cloning into 'liba'... snip Submodule path 'liba': checked out '6a0ef37c03a7068328956dcb8a08bc39f280edfc' $ git clone --recursive file://($pwd)/main.work main.home Cloning into 'main.home'... snip Submodule 'liba' (file:///tmp/gittest/work/liba.git) registered for path 'liba' Cloning into 'liba'... fatal: '/tmp/gittest/work/liba.git' does not appear to be a git repository fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. Clone of 'file:///tmp/gittest/work/liba.git' into submodule path 'liba' failed After some trial and error I managed to get what I wanted to achieve by using ./liba as the submodule URL (no .git suffix!), and creating a file named liba in /tmp/gittest/repo.a/main.git (ie. the bare origin repo) with a single line in it: gitdir: ../liba.git However, I'm not sure it is the right thing, or even advisable to do so. -- 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 0/5] am state file fix with write_file() clean-up
On Mon, Aug 24, 2015 at 10:09:41AM -0700, Junio C Hamano wrote: So here is an solution based on the write_file() is primarily to produce text, so it should be able to correct the incomplete line at the end approach. This all looks good to me. The topics-in-flight compatibility stuff in patches 3 and 5 is neatly done. Usually I would just cheat and change the order of arguments to make the compiler notice such problems, but that's hard to do here because of the varargs (you cannot just bump flags to the end). -Peff -- 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 Mon, Aug 24, 2015 at 11:01 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: 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 2output 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? I think Christian meant we should check in both, i.e. git branch output 2error grep broken\.\.\.ref error ! grep broken\.\.\.ref output or something like that. That way, you are effectively specifying in the test that badly named refs must not be included in the output, so somebody who changes that in the future needs to justify why including them in the output is a good idea when updating the test. Ah! okay, I get what you're saying. Should I reroll it? Or a squash in? -- 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 0/5] am state file fix with write_file() clean-up
On Mon, Aug 24, 2015 at 11:15:55AM -0700, Junio C Hamano wrote: This all looks good to me. The topics-in-flight compatibility stuff in patches 3 and 5 is neatly done. Usually I would just cheat and change the order of arguments to make the compiler notice such problems, but that's hard to do here because of the varargs (you cannot just bump flags to the end). Actually, I think my compatibility stuff is worthless. It would not catch new callers that wants to only probe and do their own error handling by passing 0 (and besides, assert() is a shoddy way to do this---there is no guarantee that tests will trigger all the codepaths in the first place). Oh, hrm, you're right. I was focused on making sure the common 1-passers were not broken, but patch 3 does break 0-passers (obviously, because they needed updated in the same patch ;) ). And I do agree that build-time assertions are much better than run-time ones. We should deprecate and remove write_file() by renaming the one with the updated semantics to something else, possibly with a backward compatiblity thin wrapper around it that is called write_file(), or without it to force a link-time error. That sounds reasonable. Maybe format_to_file or something? -Peff -- 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 v5 2/2] worktree: add 'list' command
Michael Rappazzo rappa...@gmail.com writes:
+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);
This does not work for traditional symlinked HEAD, no?
I'd prefer to see the callback functions of for_each_worktree() not
to duplicate the logic we already have elsewhere in the system.
Such an incomplete attempt to duplication (as we see here) will lead
to unnecessary bugs (as we see here).
Conceptually, for-each-worktree should give us the worktree root
(i.e. equivalent to $GIT_WORK_TREE in the pre-multi-worktree world)
and the git directory (i.e. equivalent to $GIT_DIR in the
pre-multi-worktree world), and the callbacks should be able to do an
equivalent of
system(git --work-tree=$GIT_WORK_TREE --git-dir=$GIT_DIR cmd)
where in this particular case cmd may be symbolic-ref HEAD or
something, no?
+ 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);
Is this new command meant to be a Porcelain? This would not work as
a plumbing that produces a machine-parseable stable output.
I am not saying that it _should_; I do not know if we even need a
'list' command that is driven from an end-user script that gives
anything more than where the work trees are.
My inclination is to suggest dropping the which branch code
altogether and only give path_only behaviour.
Thanks.
--
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 v13 00/12] port tag.c to use ref-filter APIs
On Mon, Aug 24, 2015 at 10:57 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: Interdiff: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. That sounds like a buggy behaviour that we may want to fix later, though. Perhaps document it as a known bug, e.g. Currently it does not work well when used with language-specific quoting like --shell, etc. (while punting on fixing the implementation for now)? I might have misunderstood you, But based on the discussion held here (thread.gmane.org/gmane.comp.version-control.git/276140) I thought we wanted everything inside the %(align) %(end) atoms to be quoted. So I'm a little confused by what you're trying to say. -- 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 0/5] am state file fix with write_file() clean-up
Jeff King p...@peff.net writes: On Mon, Aug 24, 2015 at 10:09:41AM -0700, Junio C Hamano wrote: So here is an solution based on the write_file() is primarily to produce text, so it should be able to correct the incomplete line at the end approach. This all looks good to me. The topics-in-flight compatibility stuff in patches 3 and 5 is neatly done. Usually I would just cheat and change the order of arguments to make the compiler notice such problems, but that's hard to do here because of the varargs (you cannot just bump flags to the end). Actually, I think my compatibility stuff is worthless. It would not catch new callers that wants to only probe and do their own error handling by passing 0 (and besides, assert() is a shoddy way to do this---there is no guarantee that tests will trigger all the codepaths in the first place). We should deprecate and remove write_file() by renaming the one with the updated semantics to something else, possibly with a backward compatiblity thin wrapper around it that is called write_file(), or without it to force a link-time error. Thanks for a dose of sanity. -- 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 3/5] write_file(): introduce an explicit WRITE_FILE_GENTLY request
Junio C Hamano gits...@pobox.com writes: All callers except for two ask this function to die upon error by passing fatal=1; turn the parameter to a more generic unsigned flag bag of bits, introduce an explicit WRITE_FILE_GENTLY bit and change these two callers to pass that bit. There is a huge iffyness around one of these two oddball callers. diff --git a/setup.c b/setup.c index 5f9f07d..718f4e1 100644 --- a/setup.c +++ b/setup.c @@ -404,7 +404,7 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir) strbuf_addf(path, %s/gitfile, gitdir); if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL)) - write_file(path.buf, 0, %s\n, gitfile); + write_file(path.buf, WRITE_FILE_GENTLY, %s\n, gitfile); strbuf_release(path); } This comes from 23af91d1 (prune: strategies for linked checkouts, 2014-11-30). I cannot tell what the justification is to treat a failure to write a gitfile as a non-error event. Just a sloppy coding that lets the program go through to its finish, ignoring the harm done by possibly corrupting user repository silently? diff --git a/transport.c b/transport.c index 40692f8..e1821a4 100644 --- a/transport.c +++ b/transport.c @@ -291,7 +291,7 @@ static int write_one_ref(const char *name, const struct object_id *oid, strbuf_addstr(buf, name); if (safe_create_leading_directories(buf-buf) || - write_file(buf-buf, 0, %s\n, oid_to_hex(oid))) + write_file(buf-buf, WRITE_FILE_GENTLY, %s\n, oid_to_hex(oid))) return error(problems writing temporary file %s: %s, buf-buf, strerror(errno)); strbuf_setlen(buf, len); This one is OK, in that it is merely to give a better error diagnosis than just oh, I cannot write so I die. -- 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] describe --contains: default to HEAD when no commit-ish is given
SZEDER Gábor sze...@ira.uka.de writes:
By the way, I usually prefer a fatter 'else' clause when everything
else is equal, i.e.
if (!argc)
argv_array_push(args, HEAD); /* default to HEAD */
else {
while (*argv) {
...
}
}
because it is easy to miss tiny else-clause while reading code, but
it is harder to miss tiny then-clause. In this case, however, the
while loop can be replaced with argv_array_pushv() these days, so
perhaps
if (!argc)
argv_array_push(args, HEAD); /* default to HEAD ... */
else
argv_array_pushv(args, argv); /* or relay what we got */
or something?
Indeed, I didn't notice argv_array_pushv() being added, log tells me
it happened quite recently. I suppose with both branches becoming a
one-liner the order of them can remain what it was in the patch,
this sparing the negation from 'if (!argc)'.
Another reason to favor the way the code is illustrated for
educational purposes above is it is easier to see exceptional case
first, i.e. if we have nothing then we do this special thing, but
otherwise we do the normal thing.
But that is a much weaker preference than the preference to fatter
else; I could go either way.
v2 comes in a minute.
Thanks.
--
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 v13 00/12] port tag.c to use ref-filter APIs
Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 10:57 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: ... + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. ... I might have misunderstood you, But based on the discussion held here (thread.gmane.org/gmane.comp.version-control.git/276140) I thought we wanted everything inside the %(align) %(end) atoms to be quoted. Perhaps I misunderstood your intention in the doc. I took everything in between %(align:...) and %(end) is quoted to mean that %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) can never satisfy %(if:empty), because %(align)%(end) would expand to a string that has two single-quotes, that is not an empty string. If that is not what would happen in the branch --list enhancement, then the proposed behaviour is good, but the above documentation would need to be updated when it happens, I think. It at least is misleading. Thanks. -- 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: Minor bug with help.autocorrect.
On Mon, Aug 24, 2015 at 01:43:27AM -0400, Jeff King wrote:
I wonder if alias_lookup() and check_pager_config(), two functions
that *know* that the string they have, cmd, could be invalid and
unusable key to give to the config API, should be doing an extra
effort (e.g. call parse_key() with QUIET option and refrain from
calling git_config_get_value()). It feels that for existing callers
of parse_key(), not passing QUIET would be the right thing to do.
Of course, I am OK if git_config_get_value() and friends took the
QUIET flag and and passed it all the way down to parse_key(); that
would be a much more correct approach to address this issue (these
two callers do not have to effectively call parse_key() twice that
way), but at the same time, that would be a lot more involved
change.
Yeah, I agree these are the only two sane fixes. Plumbing the quiet flag
through does seem really invasive; every git_config_get_foo variant
would have to learn it. [...]
Here is a patch to do the first. While it seems a little gross to have
to call parse_key twice, I think it does make sense. The alias.* and
pager.* config trees are mis-designed, and we are papering over the
problem for historical reasons.
-- 8 --
Subject: config: silence warnings for command names with invalid keys
When we are running the git command foo, we may have to
look up the config keys pager.foo and alias.foo. These
config schemes are mis-designed, as the command names can be
anything, but the config syntax has some restrictions. For
example:
$ git foo_bar
error: invalid key: pager.foo_bar
error: invalid key: alias.foo_bar
git: 'foo_bar' is not a git command. See 'git --help'.
You cannot name an alias with an underscore. And if you have
an external command with one, you cannot configure its
pager.
In the long run, we may develop a different config scheme
for these features. But in the near term (and because we'll
need to support the existing scheme indefinitely), we should
at least squelch the error messages shown above.
These errors come from git_config_parse_key. Ideally we
would pass a quiet flag to the config machinery, but there
are many layers between the pager code and the key parsing.
Passing a flag through all of those would be an invasive
change.
Instead, let's provide a config function to report on
whether a key is syntactically valid, and have the pager and
alias code skip lookup for bogus keys. We can build this
easily around the existing git_config_parse_key, with two
minor modifications:
1. We now handle a NULL store_key, to validate but not
write out the normalized key.
2. We accept a quiet flag to avoid writing to stderr.
This doesn't need to be a full-blown public flags
field, because we can make the existing implementation
a static helper function, keeping the mess contained
inside config.c.
Signed-off-by: Jeff King p...@peff.net
---
alias.c | 3 ++-
cache.h | 1 +
config.c | 39 +--
pager.c | 3 ++-
t/t7006-pager.sh | 9 +
5 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/alias.c b/alias.c
index 6aa164a..a11229d 100644
--- a/alias.c
+++ b/alias.c
@@ -5,7 +5,8 @@ char *alias_lookup(const char *alias)
char *v = NULL;
struct strbuf key = STRBUF_INIT;
strbuf_addf(key, alias.%s, alias);
- git_config_get_string(key.buf, v);
+ if (git_config_key_is_valid(key.buf))
+ git_config_get_string(key.buf, v);
strbuf_release(key);
return v;
}
diff --git a/cache.h b/cache.h
index 4e25271..8de519a 100644
--- a/cache.h
+++ b/cache.h
@@ -1440,6 +1440,7 @@ extern int git_config_pathname(const char **, const char
*, const char *);
extern int git_config_set_in_file(const char *, const char *, const char *);
extern int git_config_set(const char *, const char *);
extern int git_config_parse_key(const char *, char **, int *);
+extern int git_config_key_is_valid(const char *key);
extern int git_config_set_multivar(const char *, const char *, const char *,
int);
extern int git_config_set_multivar_in_file(const char *, const char *, const
char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 9fd275f..8adc15a 100644
--- a/config.c
+++ b/config.c
@@ -1848,7 +1848,7 @@ int git_config_set(const char *key, const char *value)
* baselen - pointer to int which will hold the length of the
* section + subsection part, can be NULL
*/
-int git_config_parse_key(const char *key, char **store_key, int *baselen_)
+static int git_config_parse_key_1(const char *key, char **store_key, int
*baselen_, int quiet)
{
int i, dot, baselen;
const char *last_dot = strrchr(key, '.');
@@ -1859,12 +1859,14 @@ int git_config_parse_key(const char *key, char
**store_key, int *baselen_)
*/
if (last_dot == NULL ||
Re: [PATCH v4] git-p4: fix faulty paths for case insensitive systems
Lars Schneider larsxschnei...@gmail.com writes: - Have you checked git log on our history and notice how nobody says PROBLEM: and SOLUTION: in capital letters? Don't try to be original in the form; your contributions are already original and valuable in the substance ;-) haha ok. I will make them lower case :-) I cannot tell if you are joking or not, but just in case you are serious, please check git log for recent history again. We do not mark our paragraphs with noisy labels like PROBLEM and SOLUTION, regardless of case. Typically, our description outlines the current status (which prepares the reader's mind to understand what you are going to talk about), highlight what is problematic in that current status, and then explains what change the patch does and justifies why it is the right change, in this order. So those who read your description can tell PROBLEM and SOLUTION apart without being told with labels. - I think I saw v3 yesterday. It would be nice to see a brief description of what has been updated in this version. I discovered an optimization. In v3 I fixed the paths of *all* files that are underneath of a given P4 clone path. In v4 I fix only the paths that are visible on the client via client-spec (P4 can perform partial checkouts via “client-views”). I was wondering how to convey this change. Would have been a cover letter for v4 the correct way or should I have made another commit on top of my v3 change? Often people do this with either (1) a cover letter for v4, that shows the git diff output to go from the result of applying v3 to the result of applying v4 to the same initial state; or (2) a textual description after three-dash line of v4 that explains what has changed relative to v3. The latter is often done when the change between v3 and v4 is small enough. Yes, that is PEP-8 style and I will change it accordingly. Unfortunately git-p4.py does not follow a style guide. e.g. line 2369: def commit(self, details, files, branch, parent = ): OK, just as I suspected. Then do not worry too much about it for now, as fixes to existing style violations should be done outside of this change, perhaps after the dust settles (or if you prefer, you can do so as a preliminary clean-up patch, that does not change anything but style, and then build your fix on top of it). More annoyingly (to me at least) is that git-p4 mixes CamelCase with snake_case even within classes/functions. I think I read somewhere that these kind of refactorings are discouraged. I assume that applies here, too? If you are doing something other than style fixes (call that meaningful work), it is strongly discouraged to fix existing style violations in the same commit. If you are going to do meaningful work on an otherwise dormant part of the system (you can judge it by checking the recent history of the files you are going to touch, e.g. git log --no-merges pu -- git-p4.py), you are encouraged to first do the style fixes in separate patches as preliminary clean-ups without changing anything else and then build your meaningful work on top of it. What is discouraged is a change that tries to only do style fixes etc. to parts of the system that are actively being modified by other people for their meaningful work. You are verifying what the set of canonical paths should be by checking ls-files output. I think that is what you intended to do (i.e. I am saying I think the code is more correct than the earlier round that used find), but I just am double checking. I agree that “ls-files” is better because it reflects what ends up in the Git repository and how it ends up there. Thanks. I wanted to double-check that the problem you saw was not about what is left in the filesystem but more about what is recorded in the Git history. ls-files check is absolutely the right approach in that case. -- 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: Minor bug with help.autocorrect.
Jeff King p...@peff.net writes: Yeah, I agree these are the only two sane fixes. Plumbing the quiet flag through does seem really invasive; every git_config_get_foo variant would have to learn it. Probably it's too gross to have a global like: config_lax_mode = 1; git_config_get_string(key.buf, v); config_lax_mode = 0; That makes a nice tidy patch, but I have a feeling we would regret it later. :) Yeah, I do think the double-checking the patch in your follow-up does is not so bad. Thanks for following it through (now I must remember not to drop these patches ;-). -- 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] am: terminate state files with a newline
On Sun, Aug 23, 2015 at 11:48:44PM -0700, Junio C Hamano wrote: I think the if here is redundant; strbuf_complete_line already handles it. True. And I like your write_state_bool() wrapper (which should be static void to the builtin/am.c) very much. On top of that, I think the right thing to do to write_file() would be to first clean-up the second parameter fatal to an unsigned flags whose (10) bit is fatal, (11) bit is binary, and make this new call to strbuf_complete_line() only when binary bit is not set. The new comment I added before write_file() function needs to be adjusted if we were to do this, obviously. Yup, I agree with all of that. I'm about to go to bed, so I'll assume you or Paul will cook up a patch. :) -Peff -- 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] am: terminate state files with a newline
Jeff King p...@peff.net writes:
FWIW, I had a similar thought when reading the original thread. I also
noted that all of the callers here pass 1 for the fatal parameter,
and that they are either bools or single strings. I wonder if:
void write_state_bool(struct am_state *state, const char *name, int v)
{
write_file(am_path(state, name), 1, %s\n, v ? t : f);
}
would make the call-sites even easier to read (and of course the \n
would be dropped here if it does migrate up to write_file()).
@@ -634,6 +641,9 @@ int write_file(const char *path, int fatal, const char
*fmt, ...)
va_start(params, fmt);
strbuf_vaddf(sb, fmt, params);
va_end(params);
+if (sb.len)
+strbuf_complete_line(sb);
+
I think the if here is redundant; strbuf_complete_line already handles
it.
True. And I like your write_state_bool() wrapper (which should be
static void to the builtin/am.c) very much.
On top of that, I think the right thing to do to write_file() would
be to first clean-up the second parameter fatal to an unsigned
flags whose (10) bit is fatal, (11) bit is binary, and make
this new call to strbuf_complete_line() only when binary bit is
not set.
The new comment I added before write_file() function needs to be
adjusted if we were to do this, obviously.
--
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 v4] git-p4: fix faulty paths for case insensitive systems
On 24 Aug 2015, at 08:33, Junio C Hamano gits...@pobox.com wrote: Lars Schneider larsxschnei...@gmail.com writes: - Have you checked git log on our history and notice how nobody says PROBLEM: and SOLUTION: in capital letters? Don't try to be original in the form; your contributions are already original and valuable in the substance ;-) haha ok. I will make them lower case :-) I cannot tell if you are joking or not, but just in case you are serious, please check git log for recent history again. We do not mark our paragraphs with noisy labels like PROBLEM and SOLUTION, regardless of case. Typically, our description outlines the current status (which prepares the reader's mind to understand what you are going to talk about), highlight what is problematic in that current status, and then explains what change the patch does and justifies why it is the right change, in this order. So those who read your description can tell PROBLEM and SOLUTION apart without being told with labels. I wasn’t joking. I got your point and I am going to change it. Sorry for the confusion. - I think I saw v3 yesterday. It would be nice to see a brief description of what has been updated in this version. I discovered an optimization. In v3 I fixed the paths of *all* files that are underneath of a given P4 clone path. In v4 I fix only the paths that are visible on the client via client-spec (P4 can perform partial checkouts via “client-views”). I was wondering how to convey this change. Would have been a cover letter for v4 the correct way or should I have made another commit on top of my v3 change? Often people do this with either (1) a cover letter for v4, that shows the git diff output to go from the result of applying v3 to the result of applying v4 to the same initial state; or (2) a textual description after three-dash line of v4 that explains what has changed relative to v3. The latter is often done when the change between v3 and v4 is small enough. Ok. Thanks! Yes, that is PEP-8 style and I will change it accordingly. Unfortunately git-p4.py does not follow a style guide. e.g. line 2369: def commit(self, details, files, branch, parent = ): OK, just as I suspected. Then do not worry too much about it for now, as fixes to existing style violations should be done outside of this change, perhaps after the dust settles (or if you prefer, you can do so as a preliminary clean-up patch, that does not change anything but style, and then build your fix on top of it). More annoyingly (to me at least) is that git-p4 mixes CamelCase with snake_case even within classes/functions. I think I read somewhere that these kind of refactorings are discouraged. I assume that applies here, too? If you are doing something other than style fixes (call that meaningful work), it is strongly discouraged to fix existing style violations in the same commit. If you are going to do meaningful work on an otherwise dormant part of the system (you can judge it by checking the recent history of the files you are going to touch, e.g. git log --no-merges pu -- git-p4.py), you are encouraged to first do the style fixes in separate patches as preliminary clean-ups without changing anything else and then build your meaningful work on top of it. What is discouraged is a change that tries to only do style fixes etc. to parts of the system that are actively being modified by other people for their meaningful work. Ok. Thanks for the explanation. You are verifying what the set of canonical paths should be by checking ls-files output. I think that is what you intended to do (i.e. I am saying I think the code is more correct than the earlier round that used find), but I just am double checking. I agree that “ls-files” is better because it reflects what ends up in the Git repository and how it ends up there. Thanks. I wanted to double-check that the problem you saw was not about what is left in the filesystem but more about what is recorded in the Git history. ls-files check is absolutely the right approach in that case. Cool! -- 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
Hi Peff, On 2015-08-23 19:43, Jeff King wrote: On Sat, Aug 22, 2015 at 05:14:39PM +0200, 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`. Hrm. This is supposed to be made non-NULL by calling init_display_notes. The git-log code does this properly, and can show notes. The rev-list code does not, and hits this assert. But that also means that it cannot actually _show_ notes, and your patch is papering over the problem. Good point... it looks like [patch] is not enough to convince the pretty-printer to actually show notes (I suspect we need something like the pretty_ctx-notes_message setup that is in show_log()). I don't know how deeply anybody _cares_ about showing notes via rev-list. It has clearly never worked. But rather than silently accepting --show-notes (and not showing any notes!), perhaps we should tell the user that it does not work. Likewise, the %N --format specifier never expands in rev-list, and should probably be removed from the rev-list documentation. Hmpf. I really did not want to uncover yet another rabbit hole... Reported in https://github.com/msysgit/git/issues/363. After reading your subject, I wondered why git rev-list --show-notes HEAD did not crash for me (whether or not I had notes). But the key element from that issue is the addition of --grep, which is what triggers us to actually look at the notes (since rev-list otherwise does not support displaying them). And that _does_ work with my patch above. So perhaps that is useful, though again, it has never worked in the past (and with your patch, it would silently return no results, whether the grep matched or not). Of course it's a terrible interface to make --show-notes --grep grep the notes, but not actually _show_ them. :-/ It is. So I'm not really in favor of this approach, but if we do go that route, the comment above the declaration of format_display_notes needs to be updated. You're right. I think the best approach for now is to error out when `--show-notes` is passed to rev-list. Do you agree? Ciao, Dscho -- 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
index file list files not found in working tree
Hi, After several merges and rebases I finally got my branches and history to reflect valid commits and proper history. Everything is pushed to internal bare repo and the remotes seems OK. When I clone the updated repository, all branches reflect the correct updated trees and blobs. The problem occurs only on the original local repository where all the merging and re-basing took place! When I checkout a branch, several files and folders are deleted from the working tree. When I examine the history of these files, there are only commits of adding them and modifying them but no log for deleting them, and they aren't deleted when I checkout the same branch in another fresh cloned repo. Git status command doesn't indicate any changes in these files. I found the files and folders names in the `.git/index` file. So after manually removing the `.git/index` file and usinge `git reset` command, `git status` indicates that the files and folders are deleted. I use `git checkout -- File_or_folder_names...` and restore all missing files and folders, just then the working tree matches the fresh checkout of the same branch on any other cloned repo. After examining the tree object of the current commit, all files and folders exists, although clearly the checkout missed some of them! Because the repository is local and private, I can't share any url for publicly accessible repository, and if one exists, no problem could be found, because the problem resides in just this certain local clone. Answering the following questions might give some clues for the problem: * How does git populate the index file after every branch checkout? * Is there any object to reflect the content of the index file? I would appreciate any pointers for where the problem could be. Thanks, Rafik -- 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 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! Your welcome + sure :) The server is on Linux, and is case-sensitive. For whatever reason (probably people committing changes on Windows in the first place) We have many P4 servers. Some run on Linux and some on Windows. The Linux ones are executed with “-C1” flag to Force the server to operate in case-insensitive mode on a normally case-sensitive platform.”. I did that in the test case, too. 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. You are correct with “P4 repo that has differences in path case”. But it can be any path case variation. Not only all upper-case. I just realized that all my examples and tests show all upper-case. I will fix that. Consider these files in P4: //depot/Path/File1 //depot/PATH/File2 //depot/pATH/File3 P4 would sync them on a case insensitive filesystem to: $CLIENT_DIR/Path/File1 $CLIENT_DIR/Path/File2 $CLIENT_DIR/Path/File3 … and this is how they should end up in Git. 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 Correct. Although the correct path might be the following too: $GIT_DIR/PATH/File1 $GIT_DIR/PATH/File2 (Am I right in thinking that this change folds the directory, but not the filename?) Correct. 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? The result is not necessarily all lowercase. Even though the case doesn’t matter as we are talking about case-insensitve filesystems I want to use the case that P4 would pick (see example above). 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? Correct. For a large repo, this approach is surely going to be really slow. True. But we use git-p4 as a one time operation to migrate our repos from P4 to Git. Therefore correctness is more important than speed. Plus it is not enabled by default. You need to pass a parameter switch to git-p4. Additionally, could you update your patch to add some words to Documentation/git-p4.txt please? I will do that! On 21 August 2015 at 18:19, larsxschnei...@gmail.com wrote: From: Lars Schneider larsxschnei...@gmail.com 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. core.ignorecase doesn’t help. I added a test case to prove that. 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 larsxschnei...@gmail.com --- 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: +
Re: git-remote-helper behavior on Windows, not recognizing blank line as terminator
On Sun, Aug 23, 2015 at 11:40:17AM -0700, Anish Athalye wrote: I'm having some issues with git remote helper behavior on Windows. According to the protocol (https://www.kernel.org/pub/software/scm/git/docs/gitremote-helpers.html), when doing things like listing capabilities, git expects the remote helper to send back a blank line when it's done. I'm having trouble having git recognize the blank line (see https://github.com/anishathalye/git-remote-dropbox/issues/13#issuecomment-133894730 for details). Has anyone come across this behavior before? Am I doing something wrong, or could there be a bug in git? What's the best way to proceed? Any help or suggestions would be greatly appreciated! The remote-helper parser tends to be very strict about its input. I suspect that on Windows you are sending CRLF rather than LF, so Git sees a line containing CR. By default the stdio streams are probably open in text mode, which will convert \n to \r\n. You probably need to reopen stdout in binary mode to make sure the output is correct. -- 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: OS X Yosemite make all doc fails
I tried building git on a fresh install of OS X Yosemite. Unfortunately it errors out. I've attached the error below as well as cat the xml/catalog file. Please let me know if there is anything else needed. Thanks in advance. Jeff OS X Yosemite 10.10.5 Xcode 6.4 (6E35b) git commit ff86faf2fa02bc21933c9e1dcc75c8d81b3e104a Merge: 8f8d0ec 552a736 Author: Junio C Hamano gits...@pobox.com Date: Wed Aug 19 14:49:37 2015 -0700 Sync with maint * maint: Start preparing for 2.5.1 $ brew install autoconf $ brew install asciidoc $ brew install docbook $ brew docbook-xsl $ git clone https://github.com/git/git $ cd git $ make configure $ ./configure --prefix=/usr $ make all doc XSLTPROC user-manual.html warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/l10n.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 29 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/l10n.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/utility.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 31 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/utility.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/labels.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 32 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/labels.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/autotoc.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 39 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/autotoc.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/verbatim.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 43 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/verbatim.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/formal.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 46 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/formal.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/table.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/table.xsl line 11 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/table.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/footnote.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 51 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/footnote.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/block.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 65 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/block.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/stripns.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 76 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/stripns.xsl make[1]: *** [user-manual.html] Error 5 make: *** [doc] Error 2 $ cat /usr/local/etc/xml/catalog ?xml version=1.0? !DOCTYPE catalog PUBLIC -//OASIS//DTD Entity Resolution XML Catalog V1.0//EN http://www.oasis-open.org/committees/entity/release/1.0/catalog.dtd; catalog xmlns=urn:oasis:names:tc:entity:xmlns:xml:catalog nextCatalog catalog=file:///usr/local/Cellar/docbook/5.0/docbook/xml/4.2/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook/5.0/docbook/xml/4.1.2/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook/5.0/docbook/xml/4.3/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook/5.0/docbook/xml/4.4/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook/5.0/docbook/xml/4.5/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook/5.0/docbook/xml/5.0/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook-xsl/1.78.1/docbook-xsl/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook-xsl/1.78.1/docbook-xsl-ns/catalog.xml/ /catalog -- To unsubscribe from this list:
Re: [RFC PATCH 2/3] run-commands: add an async queue processor
Stefan Beller sbel...@google.com writes:
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 3f10840..159ee36 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -11,6 +11,7 @@
#include exec_cmd.h
#include streaming.h
#include thread-utils.h
+#include run-command.h
static const char index_pack_usage[] =
git index-pack [-v] [-o index-file] [--keep | --keep=msg]
[--verify] [--strict] (pack-file | --stdin [--fix-thin]
[pack-file]);
@@ -1075,7 +1076,7 @@ static void resolve_base(struct object_entry *obj)
}
#ifndef NO_PTHREADS
-static void *threaded_second_pass(void *data)
+static int threaded_second_pass(struct task_queue *tq, void *data)
{
set_thread_data(data);
for (;;) {
@@ -1096,7 +1097,7 @@ static void *threaded_second_pass(void *data)
resolve_base(objects[i]);
}
- return NULL;
+ return 0;
}
#endif
@@ -1195,18 +1196,18 @@ static void resolve_deltas(void)
nr_ref_deltas + nr_ofs_deltas);
#ifndef NO_PTHREADS
- nr_dispatched = 0;
if (nr_threads 1 || getenv(GIT_FORCE_THREADS)) {
+ nr_dispatched = 0;
init_thread();
- for (i = 0; i nr_threads; i++) {
- int ret = pthread_create(thread_data[i].thread, NULL,
-threaded_second_pass,
thread_data + i);
- if (ret)
- die(_(unable to create thread: %s),
- strerror(ret));
- }
+
+ tq = create_task_queue(nr_threads);
+
for (i = 0; i nr_threads; i++)
- pthread_join(thread_data[i].thread, NULL);
+ add_task(tq, threaded_second_pass, thread_data + i);
+
+ if (finish_task_queue(tq))
+ die(Not all threads have finished);
+
cleanup_thread();
return;
}
This looks quite straight-forward, but that is not too surprising,
as the dispatcher side naturally should have a similar logic to
manage threads by creating and joining them ;-)
@@ -1075,28 +1067,24 @@ static void resolve_base(struct object_entry *obj)
}
#ifndef NO_PTHREADS
-static void *threaded_second_pass(void *data)
+static int threaded_second_pass(struct task_queue *tq, void *data)
{
- set_thread_data(data);
- for (;;) {
- int i;
- counter_lock();
- display_progress(progress, nr_resolved_deltas);
- counter_unlock();
- work_lock();
- while (nr_dispatched nr_objects
- is_delta_type(objects[nr_dispatched].type))
- nr_dispatched++;
- if (nr_dispatched = nr_objects) {
- work_unlock();
- break;
- }
- i = nr_dispatched++;
- work_unlock();
+ if (!get_thread_data()) {
+ struct thread_local *t = xmalloc(sizeof(*t));
+ t-pack_fd = open(curr_pack, O_RDONLY);
+ if (t-pack_fd == -1)
+ die_errno(_(unable to open %s), curr_pack);
- resolve_base(objects[i]);
+ set_thread_data(t);
+ /* TODO: I haven't figured out where to free this memory */
Sorry but it is hard to grok what is going on in the code with
squished indentation.
Why did I not pick upload-pack?
I could not spot easily how to make it a typical queuing problem.
We start in the threads, and once in a while we're like: Uhg, this
thread has more load than the other, let's shove a bit over there
So what we would need there is splitting the work in smallest chunks
from the beginning and just load it into the queue via add_task
... or a way for the overload and tasks to communicate with each
other and rebalance. If I am not mistaken, it has a big negative
consequence for pack-objects to split the work to too small a chunk,
as the chunk boundary also becomes boundary of find delta bases.
--
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] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider larsxschnei...@gmail.com 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 and clone the code via client spec //depot/path/... then all files not matching the case in the client spec will be ignored (in the example above file2). This is correct if core.ignorecase=false but not otherwise. Signed-off-by: Lars Schneider larsxschnei...@gmail.com --- git-p4.py | 7 ++ t/t9821-git-p4-path-variations.sh | 200 ++ 2 files changed, 207 insertions(+) create mode 100755 t/t9821-git-p4-path-variations.sh diff --git a/git-p4.py b/git-p4.py index 073f87b..0093fa3 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1950,10 +1950,14 @@ class View(object): if unmap in res: # it will list all of them, but only one not unmap-ped continue +if gitConfigBool(core.ignorecase): +res['depotFile'] = res['depotFile'].lower() self.client_spec_path_cache[res['depotFile']] = self.convert_client_path(res[clientFile]) # not found files or unmap files set to for depotFile in fileArgs: +if gitConfigBool(core.ignorecase): +depotFile = depotFile.lower() if depotFile not in self.client_spec_path_cache: self.client_spec_path_cache[depotFile] = @@ -1962,6 +1966,9 @@ class View(object): depot file should live. Returns if the file should not be mapped in the client. +if gitConfigBool(core.ignorecase): +depot_path = depot_path.lower() + if depot_path in self.client_spec_path_cache: return self.client_spec_path_cache[depot_path] diff --git a/t/t9821-git-p4-path-variations.sh b/t/t9821-git-p4-path-variations.sh new file mode 100755 index 000..599d16c --- /dev/null +++ b/t/t9821-git-p4-path-variations.sh @@ -0,0 +1,200 @@ +#!/bin/sh + +test_description='Clone repositories with path case variations' + +. ./lib-git-p4.sh + +test_expect_success 'start p4d with case folding enabled' ' + start_p4d -C1 +' + +test_expect_success 'Create a repo with path case variations' ' + client_view //depot/... //client/... + ( + cd $cli + + mkdir -p One/two + One/two/File2.txt + p4 add One/two/File2.txt + p4 submit -d Add file2 + rm -rf One + + mkdir -p one/TWO + one/TWO/file1.txt + p4 add one/TWO/file1.txt + p4 submit -d Add file1 + rm -rf one + + mkdir -p one/two + one/two/file3.txt + p4 add one/two/file3.txt + p4 submit -d Add file3 + rm -rf one + + mkdir -p outside-spec + outside-spec/file4.txt + p4 add outside-spec/file4.txt + p4 submit -d Add file4 + rm -rf outside-spec + ) +' + +test_expect_success 'Clone root' ' + client_view //depot/... //client/... + test_when_finished cleanup_git + ( + cd $git + git init . + git config core.ignorecase false + git p4 clone --use-client-spec --destination=$git //depot + # This method is used instead of test -f to ensure the case is + # checked even if the test is executed on case-insensitive file systems. + # All files are there as expected although the path cases look random. + cat expect -\EOF + One/two/File2.txt + one/TWO/file1.txt + one/two/file3.txt + outside-spec/file4.txt + EOF + git ls-files actual + test_cmp expect actual + ) +' + +test_expect_success 'Clone root (ignorecase)' ' + client_view //depot/... //client/... + test_when_finished cleanup_git + ( + cd $git + git init . + git config core.ignorecase true + git p4 clone --use-client-spec --destination=$git //depot + # This method is used instead of test -f to ensure the case is + # checked even if the test is executed on case-insensitive file systems. + # All files are there as expected although the path cases look random. + cat expect -\EOF + one/TWO/File2.txt + one/TWO/file1.txt + one/TWO/file3.txt + outside-spec/file4.txt + EOF
[PATCH v5] git-p4: Obey core.ignorecase when using P4 client specs.
From: Lars Schneider larsxschnei...@gmail.com Thanks to Luke Diamand I realized the core problem and propose here a substiantially simpler fix to my PATCH v4. The test cases remain and prove the problem. In particular 8 - Clone path (ignorecase) and Add a new file and clone path with new file (ignorecase) fail with the current implementation on OS X and Linux. Lars Schneider (1): git-p4: Obey core.ignorecase when using P4 client specs. git-p4.py | 7 ++ t/t9821-git-p4-path-variations.sh | 200 ++ 2 files changed, 207 insertions(+) create mode 100755 t/t9821-git-p4-path-variations.sh -- 1.9.5 (Apple Git-50.3) -- 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 0/5] am state file fix with write_file() clean-up
Jeff King p...@peff.net writes: On Mon, Aug 24, 2015 at 11:15:55AM -0700, Junio C Hamano wrote: This all looks good to me. The topics-in-flight compatibility stuff in patches 3 and 5 is neatly done. Usually I would just cheat and change the order of arguments to make the compiler notice such problems, but that's hard to do here because of the varargs (you cannot just bump flags to the end). Actually, I think my compatibility stuff is worthless. It would not catch new callers that wants to only probe and do their own error handling by passing 0 (and besides, assert() is a shoddy way to do this---there is no guarantee that tests will trigger all the codepaths in the first place). Oh, hrm, you're right. I was focused on making sure the common 1-passers were not broken, but patch 3 does break 0-passers (obviously, because they needed updated in the same patch ;) ). And I do agree that build-time assertions are much better than run-time ones. We should deprecate and remove write_file() by renaming the one with the updated semantics to something else, possibly with a backward compatiblity thin wrapper around it that is called write_file(), or without it to force a link-time error. That sounds reasonable. Maybe format_to_file or something? I am going into a slightly different tangent. Binary support is not something we need right now, so I'll keep the door open for that in the future by - drop the int fatal altogether from write_file() without adding unsigned flags; - add write_file_gently(), again without unsigned flags; - make them call write_file_v() that takes unsigned flags and va_list. I earlier said there were 2 oddball callers, one of them being suspicious, but it turns out that there are 3 callers that want write_file_gently(). Only the one in the setup codepath is asking for fatal=0 while discarding the error return, which is suspicious; other two are handling an error themselves and are OK. -- 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 v2 5/6] write_file(): drop caller-supplied LF from calls to create a one-liner file
All of the callsites covered by this change call write_file() or write_file_gently() to create a one-liner file. Drop the caller supplied LF and let these callees to append it as necessary. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/init-db.c | 2 +- builtin/worktree.c | 10 +- daemon.c | 2 +- setup.c| 2 +- submodule.c| 2 +- transport.c| 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/init-db.c b/builtin/init-db.c index bfe1d08..69323e1 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -378,7 +378,7 @@ static void separate_git_dir(const char *git_dir) die_errno(_(unable to move %s to %s), src, git_dir); } - write_file(git_link, gitdir: %s\n, git_dir); + write_file(git_link, gitdir: %s, git_dir); } int init_db(const char *template_dir, unsigned int flags) diff --git a/builtin/worktree.c b/builtin/worktree.c index 368502d..bbb169a 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -213,7 +213,7 @@ static int add_worktree(const char *path, const char **child_argv) * after the preparation is over. */ strbuf_addf(sb, %s/locked, sb_repo.buf); - write_file(sb.buf, initializing\n); + write_file(sb.buf, initializing); strbuf_addf(sb_git, %s/.git, path); if (safe_create_leading_directories_const(sb_git.buf)) @@ -223,8 +223,8 @@ static int add_worktree(const char *path, const char **child_argv) strbuf_reset(sb); strbuf_addf(sb, %s/gitdir, sb_repo.buf); - write_file(sb.buf, %s\n, real_path(sb_git.buf)); - write_file(sb_git.buf, gitdir: %s/worktrees/%s\n, + write_file(sb.buf, %s, real_path(sb_git.buf)); + write_file(sb_git.buf, gitdir: %s/worktrees/%s, real_path(get_git_common_dir()), name); /* * This is to keep resolve_ref() happy. We need a valid HEAD @@ -241,10 +241,10 @@ static int add_worktree(const char *path, const char **child_argv) die(_(unable to resolve HEAD)); strbuf_reset(sb); strbuf_addf(sb, %s/HEAD, sb_repo.buf); - write_file(sb.buf, %s\n, sha1_to_hex(rev)); + write_file(sb.buf, %s, sha1_to_hex(rev)); strbuf_reset(sb); strbuf_addf(sb, %s/commondir, sb_repo.buf); - write_file(sb.buf, ../..\n); + write_file(sb.buf, ../..); fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name); diff --git a/daemon.c b/daemon.c index 9154509..f9eb296 100644 --- a/daemon.c +++ b/daemon.c @@ -1376,7 +1376,7 @@ int main(int argc, char **argv) sanitize_stdfds(); if (pid_file) - write_file(pid_file, %PRIuMAX\n, (uintmax_t) getpid()); + write_file(pid_file, %PRIuMAX, (uintmax_t) getpid()); /* prepare argv for serving-processes */ cld_argv = xmalloc(sizeof (char *) * (argc + 2)); diff --git a/setup.c b/setup.c index feb8565..a206781 100644 --- a/setup.c +++ b/setup.c @@ -404,7 +404,7 @@ static void update_linked_gitdir(const char *gitfile, const char *gitdir) strbuf_addf(path, %s/gitfile, gitdir); if (stat(path.buf, st) || st.st_mtime + 24 * 3600 time(NULL)) - write_file_gently(path.buf, %s\n, gitfile); + write_file_gently(path.buf, %s, gitfile); strbuf_release(path); } diff --git a/submodule.c b/submodule.c index 5519f11..4549c1b 100644 --- a/submodule.c +++ b/submodule.c @@ -1103,7 +1103,7 @@ void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir) /* Update gitfile */ strbuf_addf(file_name, %s/.git, work_tree); - write_file(file_name.buf, gitdir: %s\n, + write_file(file_name.buf, gitdir: %s, relative_path(git_dir, real_work_tree, rel_path)); /* Update core.worktree setting */ diff --git a/transport.c b/transport.c index 0254394..788cf20 100644 --- a/transport.c +++ b/transport.c @@ -291,7 +291,7 @@ static int write_one_ref(const char *name, const struct object_id *oid, strbuf_addstr(buf, name); if (safe_create_leading_directories(buf-buf) || - write_file_gently(buf-buf, %s\n, oid_to_hex(oid))) + write_file_gently(buf-buf, %s, oid_to_hex(oid))) return error(problems writing temporary file %s: %s, buf-buf, strerror(errno)); strbuf_setlen(buf, len); -- 2.5.0-568-g53a3e28 -- 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 v2 6/6] write_file(): drop caller-supplied LF from multi-line file
This is just to illustrate that we _could_ do this; I think it is
better to leave these places as they are. The primary thing we
wanted to do with the automatic addition of LF to an incomplete line
was to make it easier to write a caller that creates a single-liner
file. For callers that want to fully create a multi-line input, the
resulting code is easier to see if we let them continue to do so.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 1 -
builtin/branch.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 486ff59..c544091 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -381,7 +381,6 @@ static void write_author_script(const struct am_state
*state)
strbuf_addstr(sb, GIT_AUTHOR_DATE=);
sq_quote_buf(sb, state-author_date);
- strbuf_addch(sb, '\n');
write_state_text(state, author-script, sb.buf);
diff --git a/builtin/branch.c b/builtin/branch.c
index ff05869..cdf7f13 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -774,7 +774,7 @@ static int edit_branch_description(const char *branch_name)
strbuf_commented_addf(buf,
Please edit the description for the branch\n
%s\n
- Lines starting with '%c' will be stripped.\n,
+ Lines starting with '%c' will be stripped.,
branch_name, comment_line_char);
if (write_file_gently(git_path(edit_description), %s, buf.buf)) {
strbuf_release(buf);
--
2.5.0-568-g53a3e28
--
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 v2 3/6] write_file(): drop fatal parameter
All callers except three passed 1 for the fatal parameter to ask
this function to die upon error, but to a casual reader of the code,
it was not all obvious what that 1 meant. Instead, split the
function into two based on a common write_file_v() that takes the
flag, introduce write_file_gently() as a new way to attempt creating
a file without dying on error, and make three callers to call it.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 4 ++--
builtin/branch.c | 2 +-
builtin/init-db.c | 2 +-
builtin/worktree.c | 10 +-
cache.h| 5 +++--
daemon.c | 2 +-
setup.c| 2 +-
submodule.c| 2 +-
transport.c| 2 +-
wrapper.c | 28
10 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index f0a046b..9c57677 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -205,13 +205,13 @@ static int write_state_text(const struct am_state *state,
fmt = %s\n;
else
fmt = %s;
- return write_file(am_path(state, name), 1, fmt, string);
+ return write_file(am_path(state, name), fmt, string);
}
static int write_state_count(const struct am_state *state,
const char *name, int value)
{
- return write_file(am_path(state, name), 1, %d\n, value);
+ return write_file(am_path(state, name), %d\n, value);
}
static int write_state_bool(const struct am_state *state,
diff --git a/builtin/branch.c b/builtin/branch.c
index 58aa84f..ff05869 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -776,7 +776,7 @@ static int edit_branch_description(const char *branch_name)
%s\n
Lines starting with '%c' will be stripped.\n,
branch_name, comment_line_char);
- if (write_file(git_path(edit_description), 0, %s, buf.buf)) {
+ if (write_file_gently(git_path(edit_description), %s, buf.buf)) {
strbuf_release(buf);
return error(_(could not write branch description template:
%s),
strerror(errno));
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 49df78d..bfe1d08 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -378,7 +378,7 @@ static void separate_git_dir(const char *git_dir)
die_errno(_(unable to move %s to %s), src, git_dir);
}
- write_file(git_link, 1, gitdir: %s\n, git_dir);
+ write_file(git_link, gitdir: %s\n, git_dir);
}
int init_db(const char *template_dir, unsigned int flags)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 6a264ee..368502d 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -213,7 +213,7 @@ static int add_worktree(const char *path, const char
**child_argv)
* after the preparation is over.
*/
strbuf_addf(sb, %s/locked, sb_repo.buf);
- write_file(sb.buf, 1, initializing\n);
+ write_file(sb.buf, initializing\n);
strbuf_addf(sb_git, %s/.git, path);
if (safe_create_leading_directories_const(sb_git.buf))
@@ -223,8 +223,8 @@ static int add_worktree(const char *path, const char
**child_argv)
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/worktrees/%s\n,
+ write_file(sb.buf, %s\n, real_path(sb_git.buf));
+ write_file(sb_git.buf, gitdir: %s/worktrees/%s\n,
real_path(get_git_common_dir()), name);
/*
* This is to keep resolve_ref() happy. We need a valid HEAD
@@ -241,10 +241,10 @@ static int add_worktree(const char *path, const char
**child_argv)
die(_(unable to resolve HEAD));
strbuf_reset(sb);
strbuf_addf(sb, %s/HEAD, sb_repo.buf);
- write_file(sb.buf, 1, %s\n, sha1_to_hex(rev));
+ write_file(sb.buf, %s\n, sha1_to_hex(rev));
strbuf_reset(sb);
strbuf_addf(sb, %s/commondir, sb_repo.buf);
- write_file(sb.buf, 1, ../..\n);
+ write_file(sb.buf, ../..\n);
fprintf_ln(stderr, _(Enter %s (identifier %s)), path, name);
diff --git a/cache.h b/cache.h
index 6bb7119..3f79e6b 100644
--- a/cache.h
+++ b/cache.h
@@ -1539,8 +1539,9 @@ static inline ssize_t write_str_in_full(int fd, const
char *str)
{
return write_in_full(fd, str, strlen(str));
}
-__attribute__((format (printf, 3, 4)))
-extern int write_file(const char *path, int fatal, const char *fmt, ...);
+
+extern int write_file(const char *path, const char *fmt, ...);
+extern int write_file_gently(const char *path, const char *fmt, ...);
/* pager.c */
extern void setup_pager(void);
diff --git a/daemon.c b/daemon.c
index d3d3e43..9154509 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1376,7 +1376,7 @@ int main(int argc, char **argv)
sanitize_stdfds();
if (pid_file)
-
[PATCH v2 2/6] builtin/am: make sure state files are text
We forgot to terminate the payload given to write_file() with LF,
resulting in files that end with an incomplete line. Teach the
wrappers builtin/am uses to make sure it adds LF at the end as
necessary.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 4d34dc5..f0a046b 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -199,13 +199,19 @@ static inline const char *am_path(const struct am_state
*state, const char *path
static int write_state_text(const struct am_state *state,
const char *name, const char *string)
{
- return write_file(am_path(state, name), 1, %s, string);
+ const char *fmt;
+
+ if (*string string[strlen(string) - 1] != '\n')
+ fmt = %s\n;
+ else
+ fmt = %s;
+ return write_file(am_path(state, name), 1, fmt, string);
}
static int write_state_count(const struct am_state *state,
const char *name, int value)
{
- return write_file(am_path(state, name), 1, %d, value);
+ return write_file(am_path(state, name), 1, %d\n, value);
}
static int write_state_bool(const struct am_state *state,
--
2.5.0-568-g53a3e28
--
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 v2 0/6] am state file fix with write_file() clean-up
git am was recently reimplemented in C. While the implementation was done conservatively and followed the original logic in the scripted version fairly faithfully, the state files it left in the $GIT_DIR/rebase-apply directory were made slightly different by mistake---they lacked the final LF, leaving their last line incomplete. The patch [1/6] is Peff's idea to consolidate callers in am, in a more concrete form. The patch [2/6] is the fix to the state files with incomplete lines. The workhorse helper function that implements we have this (short) body of text; create a new file that contains it has a fatal parameter, to which 1 was passed by almost all callers, but to casual readers, it was unclear what that 1 meant. The patch [3/6] splits it to write_file() and write_file_gently() and drops this parameter that looks mysterious at the callsites. A common helper function write_file_v() is introduced to implement these two as thin wrappers of it. The patch [4/6] updates write_file_v() so that it does the we are writing a text file. Make sure it does not end with an incomplete line logic that [2/6] added only to builtin/am.c, thusly reverting what was done to builtin/am.c in [2/6]. The patch [5/6] stops all callers that creates a single-liner file using write_file() and write_file_gently() from including the final LF to the format they pass. This should not change the behaviour, but it probably makes it conceptually cleaner. You have the contents to be placed on a single line, and the helper turns the contents into a proper line. The patch [6/6] drops the final LF from the parameter to create a multi-line file; while this does not hurt in the sense that the callee will add a necessary LF back, I do not think it should be applied. Conceptually, if you have a buffer that contains a bunch of lines and throw it at a helper to create a file, you'd better have the terminating LF yourself before asking the helper to put them in the file. Junio C Hamano (6): builtin/am: introduce write_state_*() helper functions builtin/am: make sure state files are text write_file(): drop fatal parameter write_file_v(): do not leave incomplete line at the end write_file(): drop caller-supplied LF from calls to create a one-liner file write_file(): drop caller-supplied LF from multi-line file builtin/am.c | 69 -- builtin/branch.c | 4 ++-- builtin/init-db.c | 2 +- builtin/worktree.c | 10 cache.h| 5 ++-- daemon.c | 2 +- setup.c| 2 +- submodule.c| 2 +- transport.c| 2 +- wrapper.c | 36 10 files changed, 88 insertions(+), 46 deletions(-) -- 2.5.0-568-g53a3e28 -- 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 v2 4/6] write_file_v(): do not leave incomplete line at the end
All existing callers to this function use it to produce a text file
or an empty file, and a new callsite that mimick them must end their
payload with a LF. If they forget to do so, the resulting file will
end with an incomplete line.
Introduce WRITE_FILE_BINARY flag bit, which no existing callers
pass, and unless that bit is set, make sure that write_file_v() adds
an extra LF at the end of an incomplete line as necessary.
With this, the caller-side fix in builtin/am.c becomes unnecessary.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 10 ++
wrapper.c| 14 +++---
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 9c57677..486ff59 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -199,19 +199,13 @@ static inline const char *am_path(const struct am_state
*state, const char *path
static int write_state_text(const struct am_state *state,
const char *name, const char *string)
{
- const char *fmt;
-
- if (*string string[strlen(string) - 1] != '\n')
- fmt = %s\n;
- else
- fmt = %s;
- return write_file(am_path(state, name), fmt, string);
+ return write_file(am_path(state, name), %s, string);
}
static int write_state_count(const struct am_state *state,
const char *name, int value)
{
- return write_file(am_path(state, name), %d\n, value);
+ return write_file(am_path(state, name), %d, value);
}
static int write_state_bool(const struct am_state *state,
diff --git a/wrapper.c b/wrapper.c
index 8c8925b..db39e1b 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -621,17 +621,25 @@ char *xgetcwd(void)
return strbuf_detach(sb, NULL);
}
-static int write_file_v(const char *path, int fatal,
+
+#define WRITE_FILE_GENTLY (1 0)
+#define WRITE_FILE_BINARY (1 1)
+
+static int write_file_v(const char *path, unsigned flags,
const char *fmt, va_list params)
{
+ int fatal = !(flags WRITE_FILE_GENTLY);
struct strbuf sb = STRBUF_INIT;
int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0666);
+
if (fd 0) {
if (fatal)
die_errno(_(could not open %s for writing), path);
return -1;
}
strbuf_vaddf(sb, fmt, params);
+ if (!(flags WRITE_FILE_BINARY))
+ strbuf_complete_line(sb);
if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
int err = errno;
close(fd);
@@ -656,7 +664,7 @@ int write_file(const char *path, const char *fmt, ...)
va_list params;
va_start(params, fmt);
- status = write_file_v(path, 1, fmt, params);
+ status = write_file_v(path, 0, fmt, params);
va_end(params);
return status;
}
@@ -667,7 +675,7 @@ int write_file_gently(const char *path, const char *fmt,
...)
va_list params;
va_start(params, fmt);
- status = write_file_v(path, 0, fmt, params);
+ status = write_file_v(path, WRITE_FILE_GENTLY, fmt, params);
va_end(params);
return status;
}
--
2.5.0-568-g53a3e28
--
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 v2 1/6] builtin/am: introduce write_state_*() helper functions
There are many calls to write_file() that repeat the same pattern in
the implementation of the builtin version of am. They all share
the same traits, i.e they
- produce a text file with a single string in it;
- have enough information to produce the entire contents of that
file;
- generate the pathname of the file by making a call to am_path(); and
- they ask write_file() to die() upon failure.
The slight differences among the call sites throw them into roughly
three categories:
- many write either t or f based on a boolean value to a file;
- some write the integer value in decimal text;
- some others write more general string, e.g. an object name in
hex, an empty string (i.e. the presense of the file itself serves
as a flag), etc.
Introduce three helpers, write_state_bool(), write_state_count() and
write_state_text(), to reduce direct calls to write_file().
This is a preparatory step for the next step to ensure that no
state file this command leaves in $GIT_DIR is with an incomplete
line at the end.
Suggested-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/am.c | 68
1 file changed, 41 insertions(+), 27 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 634f7a7..4d34dc5 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -194,6 +194,27 @@ static inline const char *am_path(const struct am_state
*state, const char *path
}
/**
+ * For convenience to call write_file()
+ */
+static int write_state_text(const struct am_state *state,
+ const char *name, const char *string)
+{
+ return write_file(am_path(state, name), 1, %s, string);
+}
+
+static int write_state_count(const struct am_state *state,
+const char *name, int value)
+{
+ return write_file(am_path(state, name), 1, %d, value);
+}
+
+static int write_state_bool(const struct am_state *state,
+ const char *name, int value)
+{
+ return write_state_text(state, name, value ? t : f);
+}
+
+/**
* If state-quiet is false, calls fprintf(fp, fmt, ...), and appends a newline
* at the end.
*/
@@ -362,7 +383,7 @@ static void write_author_script(const struct am_state
*state)
sq_quote_buf(sb, state-author_date);
strbuf_addch(sb, '\n');
- write_file(am_path(state, author-script), 1, %s, sb.buf);
+ write_state_text(state, author-script, sb.buf);
strbuf_release(sb);
}
@@ -1000,13 +1021,10 @@ 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, quiet), 1, state-quiet ? t : f);
-
- write_file(am_path(state, sign), 1, state-signoff ? t : f);
-
- write_file(am_path(state, utf8), 1, state-utf8 ? t : f);
+ write_state_bool(state, threeway, state-threeway);
+ write_state_bool(state, quiet, state-quiet);
+ write_state_bool(state, sign, state-signoff);
+ write_state_bool(state, utf8, state-utf8);
switch (state-keep) {
case KEEP_FALSE:
@@ -1022,9 +1040,8 @@ 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, messageid), 1, state-message_id ? t :
f);
+ write_state_text(state, keep, str);
+ write_state_bool(state, messageid, state-message_id);
switch (state-scissors) {
case SCISSORS_UNSET:
@@ -1039,24 +1056,23 @@ static void am_setup(struct am_state *state, enum
patch_format patch_format,
default:
die(BUG: invalid value for state-scissors);
}
-
- write_file(am_path(state, scissors), 1, %s, str);
+ write_state_text(state, scissors, str);
sq_quote_argv(sb, state-git_apply_opts.argv, 0);
- write_file(am_path(state, apply-opt), 1, %s, sb.buf);
+ write_state_text(state, apply-opt, sb.buf);
if (state-rebasing)
- write_file(am_path(state, rebasing), 1, %s, );
+ write_state_text(state, rebasing, );
else
- write_file(am_path(state, applying), 1, %s, );
+ write_state_text(state, applying, );
if (!get_sha1(HEAD, curr_head)) {
- write_file(am_path(state, abort-safety), 1, %s,
sha1_to_hex(curr_head));
+ write_state_text(state, abort-safety, sha1_to_hex(curr_head));
if (!state-rebasing)
update_ref(am, ORIG_HEAD, curr_head, NULL, 0,
UPDATE_REFS_DIE_ON_ERR);
} else {
- write_file(am_path(state, abort-safety), 1, %s, );
+ write_state_text(state, abort-safety, );
Re: [PATCH v13 02/12] ref-filter: introduce ref_formatting_state and ref_formatting_stack
Karthik Nayak karthik@gmail.com writes:
+static void push_new_stack_element(struct ref_formatting_stack **stack)
+{
Micronit. Perhaps s/_new//;, as you do not call the other function
pop_old_stack_element().
The remainder of this step looks pretty straight-forward and was a
pleasant read.
Thanks.
--
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 v13 04/12] ref-filter: implement an `align` atom
Karthik Nayak karthik@gmail.com writes:
+static void perform_quote_formatting(struct strbuf *s, const char *str, int
quote_style);
+
+static void end_atom_handler(struct atom_value *atomv, struct
ref_formatting_state *state)
+{
+ struct ref_formatting_stack *current = state-stack;
+ struct strbuf s = STRBUF_INIT;
+
+ if (!current-at_end)
+ die(_(format: `end` atom used without a supporting atom));
+ current-at_end(current);
+ /*
+ * Whenever we have more than one stack element that means we
+ * are using a certain modifier atom. In that case we need to
+ * perform quote formatting.
+ */
+ if (!state-stack-prev-prev) {
The comment and the condition seem to be saying opposite things.
The code says If the stack only has one prev that is the very
initial one, then we do the quoting, i.e. the result of expanding
the enclosed string in %(start-something)...%(end) is quoted only
when that appears at the top level, which feels more correct than
the comment that says if we are about to pop after seeing the
first %(end) in %(start)...%(another)...%(end)...%(end) sequence,
we quote what is between %(another)...%(end).
+ perform_quote_formatting(s, current-output.buf,
state-quote_style);
+ strbuf_reset(current-output);
+ strbuf_addbuf(current-output, s);
+ }
+ strbuf_release(s);
+ pop_stack_element(state-stack);
+}
+
@@ -1228,29 +1315,33 @@ void ref_array_sort(struct ref_sorting *sorting,
struct ref_array *array)
qsort(array-items, array-nr, sizeof(struct ref_array_item *),
compare_refs);
}
-static void append_atom(struct atom_value *v, struct ref_formatting_state
*state)
+static void perform_quote_formatting(struct strbuf *s, const char *str, int
quote_style)
{
- struct strbuf *s = state-stack-output;
-
- switch (state-quote_style) {
+ switch (quote_style) {
case QUOTE_NONE:
- strbuf_addstr(s, v-s);
+ strbuf_addstr(s, str);
break;
case QUOTE_SHELL:
- sq_quote_buf(s, v-s);
+ sq_quote_buf(s, str);
break;
case QUOTE_PERL:
- perl_quote_buf(s, v-s);
+ perl_quote_buf(s, str);
break;
case QUOTE_PYTHON:
- python_quote_buf(s, v-s);
+ python_quote_buf(s, str);
break;
case QUOTE_TCL:
- tcl_quote_buf(s, v-s);
+ tcl_quote_buf(s, str);
break;
}
}
+static void append_atom(struct atom_value *v, struct ref_formatting_state
*state)
+{
+ struct strbuf *s = state-stack-output;
+ perform_quote_formatting(s, v-s, state-quote_style);
Hmmm, do we want to unconditionally do the quote here, or only when
we are not being captured by upcoming %(end) to be consistent with
the behaviour of end_atom_handler() above?
@@ -1307,7 +1398,18 @@ void show_ref_array_item(struct ref_array_item *info,
const char *format, int qu
if (cp sp)
append_literal(cp, sp, state);
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep),
atomv);
- append_atom(atomv, state);
+ /*
+ * If the atom is a modifier atom, then call the handler
function.
+ * Else, if this is the first element on the stack, then we
need to
+ * format the atom as per the given quote. Else we just add the
atom value
+ * to the current stack element and handle quote formatting at
the end.
+ */
+ if (atomv-handler)
+ atomv-handler(atomv, state);
+ else if (!state.stack-prev)
+ append_atom(atomv, state);
+ else
+ strbuf_addstr(state.stack-output, atomv-s);
Ahh, this explains why you are not doing it above, but I do not
think if this is a good division of labor.
You can see that I expected that if !state.stack-prev check to be
inside append_atom(), and I would imagine future readers would have
the same expectation when reading this code. I.e.
append_atom(struct atom_value *v, struct ref_f_s *state)
{
if (state-stack-prev)
strbuf_addstr(state-stack-output, v-s);
else
quote_format(state-stack-output, v-s,
state-quote_style);
}
The end result may be the same, but I do think append_atom is to
always quote, so we do an unquoted appending by hand is a bad way
to do this.
Moreover, notice that the function signature of append_atom() is
exactly the same as atomv-handler's. I wonder if it would be
easier to understand if you made append_atom() the handler for a
non-magic atoms, which would let you do the above without any if/else
and just a single unconditional
atomv-handler(atomv,
Re: OS X Yosemite make all doc fails
On Mon, Aug 24, 2015 at 02:30:39AM -0700, Jeff S wrote: XSLTPROC user-manual.html warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/l10n.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 29 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/l10n.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/utility.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 31 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/utility.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/labels.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 32 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/labels.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/autotoc.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 39 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/autotoc.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/verbatim.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 43 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/verbatim.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/formal.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 46 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/formal.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/table.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/table.xsl line 11 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/table.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/footnote.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 51 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/footnote.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/html/block.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 65 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/html/block.xsl warning: failed to load external entity http://docbook.sourceforge.net/release/xsl/current/common/stripns.xsl; compilation error: file http://docbook.sourceforge.net/release/xsl/current/html/docbook.xsl line 76 element include xsl:include : unable to load http://docbook.sourceforge.net/release/xsl/current/common/stripns.xsl make[1]: *** [user-manual.html] Error 5 make: *** [doc] Error 2 It's clear from the message that your catalogs are not properly set up. Once you get that working, the documentation should build correctly. nextCatalog catalog=file:///usr/local/Cellar/docbook-xsl/1.78.1/docbook-xsl/catalog.xml/ nextCatalog catalog=file:///usr/local/Cellar/docbook-xsl/1.78.1/docbook-xsl-ns/catalog.xml/ /catalog Unfortunately, this doesn't tell us much, since these are all references to other catalogs. You need to look in the other catalogs, specifically the two mentioned above, and ensure that there are appropriate rewriteURI and rewriteSystem entries pointing to the right place. On my system, that looks like the following: rewriteURI uriStartString=http://docbook.sourceforge.net/release/xsl/current/; rewritePrefix=.// rewriteSystem systemIdStartString=http://docbook.sourceforge.net/release/xsl/current/; rewritePrefix=.// I don't know how your system is configured, so I can't tell you what entries would be correct. You might ask in an appropriate user forum for Homebrew. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Junio C Hamano gits...@pobox.com writes: Karthik Nayak karthik@gmail.com writes: On Mon, Aug 24, 2015 at 10:57 PM, Junio C Hamano gits...@pobox.com wrote: Karthik Nayak karthik@gmail.com writes: ... + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. ... I might have misunderstood you, But based on the discussion held here (thread.gmane.org/gmane.comp.version-control.git/276140) I thought we wanted everything inside the %(align) %(end) atoms to be quoted. Perhaps I misunderstood your intention in the doc. I took everything in between %(align:...) and %(end) is quoted to mean that %(if:empty)%(align)%(end)%(then)Empty%(else)Not Empty%(end) can never satisfy %(if:empty), because %(align)%(end) would expand to a string that has two single-quotes, that is not an empty string. If that is not what would happen in the branch --list enhancement, then the proposed behaviour is good, but the above documentation would need to be updated when it happens, I think. It at least is misleading. OK, now I checked the code, and I _think_ the recursive logic is doing the right thing (modulo minor nits on comment-vs-code discrepancy and code structure I sent separately). -- 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 v2 2/6] builtin/am: make sure state files are text
On Mon, Aug 24, 2015 at 01:58:06PM -0700, Junio C Hamano wrote: We forgot to terminate the payload given to write_file() with LF, resulting in files that end with an incomplete line. Teach the wrappers builtin/am uses to make sure it adds LF at the end as necessary. Is it even worth doing this step? It's completely reverted later in the series. I understand that we do not want to hold the fix to git-am hostage to write_file refactoring, but I don't see any reason these cannot all graduate as part of the same topic. Ignore me if you really are planning on doing the first two to maint and holding the others back for master. -Peff -- 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 v13 05/12] ref-filter: add option to filter out tags, branches and remotes
Karthik Nayak karthik@gmail.com writes:
From: Karthik Nayak karthik@gmail.com
Add a function called 'for_each_reftype_fullpath()' to refs.{c,h}
which iterates through each ref for the given path without trimming
the path and also accounting for broken refs, if mentioned.
For this part, I think you would want to borrow an extra pair of
eyes from Michael.
Add 'filter_ref_kind()' in ref-filter.c to check the kind of ref being
handled and return the kind to 'ref_filter_handler()', where we
discard refs which we do not need and assign the kind to needed refs.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
ref-filter.c | 59 ++-
ref-filter.h | 12 ++--
refs.c | 9 +
refs.h | 1 +
4 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index ffec10a..d5fae1a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1123,6 +1123,36 @@ static struct ref_array_item *new_ref_array_item(const
char *refname,
return ref;
}
+static int filter_ref_kind(struct ref_filter *filter, const char *refname)
+{
+ unsigned int i;
+
+ static struct {
+ const char *prefix;
+ unsigned int kind;
+ } ref_kind[] = {
+ { refs/heads/ , FILTER_REFS_BRANCHES },
+ { refs/remotes/ , FILTER_REFS_REMOTES },
+ { refs/tags/, FILTER_REFS_TAGS}
+ };
+
+ if (filter-kind == FILTER_REFS_BRANCHES)
+ return FILTER_REFS_BRANCHES;
+ else if (filter-kind == FILTER_REFS_REMOTES)
+ return FILTER_REFS_REMOTES;
+ else if (filter-kind == FILTER_REFS_TAGS)
+ return FILTER_REFS_TAGS;
+ else if (!strcmp(refname, HEAD))
+ return FILTER_REFS_DETACHED_HEAD;
+
+ for (i = 0; i ARRAY_SIZE(ref_kind); i++) {
+ if (starts_with(refname, ref_kind[i].prefix))
+ return ref_kind[i].kind;
+ }
+
+ return FILTER_REFS_OTHERS;
+}
+
/*
* A call-back given to for_each_ref(). Filter refs and keep them for
* later object processing.
@@ -1133,6 +1163,7 @@ static int ref_filter_handler(const char *refname,
const struct object_id *oid,
struct ref_filter *filter = ref_cbdata-filter;
struct ref_array_item *ref;
struct commit *commit = NULL;
+ unsigned int kind;
if (flag REF_BAD_NAME) {
warning(ignoring ref with broken name %s, refname);
@@ -1144,6 +1175,10 @@ static int ref_filter_handler(const char *refname,
const struct object_id *oid,
return 0;
}
+ kind = filter_ref_kind(filter, refname);
+ if (!(kind filter-kind))
+ return 0;
+
When filter-kind is set to some single-bit thing (e.g.
FILTER_REFS_BRANCHES) by the caller of ref_filter_handler(), then
this call of filter_ref_kind() will just parrot that without even
looking at refname. And then the if statement says yes, they have
common bit(s). Even when refname is refs/tags/v1.0 or HEAD.
And if this code knows that refs/tags/v1.0 or HEAD will never
come when filter-kind is exactly FILTER_REFS_BRANCHES, then I do
not see the point of calling filter_ref_kind().
I am not sure what this is trying to do. Can you clarify the
thinking behind this as a comment to filter_ref_kind()?
@@ -1175,6 +1210,7 @@ static int ref_filter_handler(const char *refname,
const struct object_id *oid,
REALLOC_ARRAY(ref_cbdata-array-items, ref_cbdata-array-nr + 1);
ref_cbdata-array-items[ref_cbdata-array-nr++] = ref;
+ ref-kind = kind;
return 0;
}
@@ -1251,16 +1287,29 @@ int filter_refs(struct ref_array *array, struct
ref_filter *filter, unsigned int
{
struct ref_filter_cbdata ref_cbdata;
int ret = 0;
+ unsigned int broken = 0;
ref_cbdata.array = array;
ref_cbdata.filter = filter;
/* Simple per-ref filtering */
- if (type (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
- ret = for_each_rawref(ref_filter_handler, ref_cbdata);
- else if (type FILTER_REFS_ALL)
- ret = for_each_ref(ref_filter_handler, ref_cbdata);
- else if (type)
+ if (type FILTER_REFS_INCLUDE_BROKEN) {
+ type = ~FILTER_REFS_INCLUDE_BROKEN;
+ broken = 1;
+ }
+
+ filter-kind = type;
+ if (type == FILTER_REFS_BRANCHES)
+ ret = for_each_reftype_fullpath(ref_filter_handler,
refs/heads/, broken, ref_cbdata);
+ else if (type == FILTER_REFS_REMOTES)
+ ret = for_each_reftype_fullpath(ref_filter_handler,
refs/remotes/, broken, ref_cbdata);
+ else if (type == FILTER_REFS_TAGS)
+ ret = for_each_reftype_fullpath(ref_filter_handler,
refs/tags/, broken, ref_cbdata);
+ else
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is - performed. + performed. If used with '--quote' everything in between %(align:..) + and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). I had already commented on a preliminary version of this series off-list. I think all my previous comments have been taken into account. Thanks, both. I think this is pretty close to being satisfactory ;-) There may probably be a handful of minor nits like the above that need to be addressed, but I do not think I saw anything glaringly wrong that makes the series unsalvageable. It was a very pleasant read. It's almost there, and I am very happy to see how this and other series evolved so far ;-) Having said all that, it seems that there is some trivial typo or thinko in the formatting code to break t7004. Here is what I see... ok 98 - verifying rfc1991 signature expecting success: echo rfc1991 gpghome/gpg.conf echo rfc1991-signed-tag RFC1991 signed tag expect git tag -l -n1 rfc1991-signed-tag actual test_cmp expect actual git tag -l -n2 rfc1991-signed-tag actual test_cmp expect actual git tag -l -n999 rfc1991-signed-tag actual test_cmp expect actual --- expect 2015-08-24 22:54:44.607272653 + +++ actual 2015-08-24 22:54:44.611272643 + @@ -1 +1 @@ -rfc1991-signed-tag RFC1991 signed tag +rfc1991-signed-tagRFC1991 signed tag not ok 99 - list tag with rfc1991 signature -- 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 v5 2/2] worktree: add 'list' command
On Mon, Aug 24, 2015 at 2:05 PM, Junio C Hamano gits...@pobox.com wrote:
Michael Rappazzo rappa...@gmail.com writes:
+ 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);
Is this new command meant to be a Porcelain? This would not work as
a plumbing that produces a machine-parseable stable output.
I am not saying that it _should_; I do not know if we even need a
'list' command that is driven from an end-user script that gives
anything more than where the work trees are.
My inclination is to suggest dropping the which branch code
altogether and only give path_only behaviour.
The which branch was probably added in response to this [1] review,
which suggested that at some point, we might want to provide the user
with interesting information about each worktree, such as
branch/detached head, tag, locked status (plus lock reason and whether
currently accessible), prune-able status (plus reason). This could
optionally be controlled by --verbose or some other extended
formatting option.
The same review also suggested a --porcelain option for script writers.
[1]: http://article.gmane.org/gmane.comp.version-control.git/275528
--
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: git-remote-helper behavior on Windows, not recognizing blank line as terminator
Wow, yeah, that was it. Thanks for your help! On Aug 24, 2015, at 2:24 AM, John Keeping j...@keeping.me.uk wrote: On Sun, Aug 23, 2015 at 11:40:17AM -0700, Anish Athalye wrote: I'm having some issues with git remote helper behavior on Windows. According to the protocol (https://www.kernel.org/pub/software/scm/git/docs/gitremote-helpers.html), when doing things like listing capabilities, git expects the remote helper to send back a blank line when it's done. I'm having trouble having git recognize the blank line (see https://github.com/anishathalye/git-remote-dropbox/issues/13#issuecomment-133894730 for details). Has anyone come across this behavior before? Am I doing something wrong, or could there be a bug in git? What's the best way to proceed? Any help or suggestions would be greatly appreciated! The remote-helper parser tends to be very strict about its input. I suspect that on Windows you are sending CRLF rather than LF, so Git sees a line containing CR. By default the stdio streams are probably open in text mode, which will convert \n to \r\n. You probably need to reopen stdout in binary mode to make sure the output is correct. -- 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 v2 0/6] am state file fix with write_file() clean-up
On Mon, Aug 24, 2015 at 01:58:04PM -0700, Junio C Hamano wrote: The workhorse helper function that implements we have this (short) body of text; create a new file that contains it has a fatal parameter, to which 1 was passed by almost all callers, but to casual readers, it was unclear what that 1 meant. The patch [3/6] splits it to write_file() and write_file_gently() and drops this parameter that looks mysterious at the callsites. A common helper function write_file_v() is introduced to implement these two as thin wrappers of it. To be honest, I think the flags field is more maintainable going forward. Now you have _two_ functions, and any features you add to them have to go in both places. In 4/6 you add the WRITE_FILE_BINARY flag, but I notice that callers can't actually pass it. And adding it into write_file() would take us back to square-one with source compatibility. The patch [4/6] updates write_file_v() so that it does the we are writing a text file. Make sure it does not end with an incomplete line logic that [2/6] added only to builtin/am.c, thusly reverting what was done to builtin/am.c in [2/6]. I notice this also converts fatal to flags. It seemed weird to me that did not go into patch 3, but I guess it is OK (we know that write_file_v has no outstanding callers, since we just added it). The patch [5/6] stops all callers that creates a single-liner file using write_file() and write_file_gently() from including the final LF to the format they pass. This should not change the behaviour, but it probably makes it conceptually cleaner. You have the contents to be placed on a single line, and the helper turns the contents into a proper line. Nice. The patch [6/6] drops the final LF from the parameter to create a multi-line file; while this does not hurt in the sense that the callee will add a necessary LF back, I do not think it should be applied. Conceptually, if you have a buffer that contains a bunch of lines and throw it at a helper to create a file, you'd better have the terminating LF yourself before asking the helper to put them in the file. I agree we should drop this one. -Peff -- 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 v13 04/12] ref-filter: implement an `align` atom
Junio C Hamano gits...@pobox.com writes:
Karthik Nayak karthik@gmail.com writes:
+static void end_atom_handler(struct atom_value *atomv, struct
ref_formatting_state *state)
+{
+struct ref_formatting_stack *current = state-stack;
+struct strbuf s = STRBUF_INIT;
+
+if (!current-at_end)
+die(_(format: `end` atom used without a supporting atom));
+current-at_end(current);
+/*
+ * Whenever we have more than one stack element that means we
+ * are using a certain modifier atom. In that case we need to
+ * perform quote formatting.
+ */
+if (!state-stack-prev-prev) {
The comment and the condition seem to be saying opposite things.
The code says If the stack only has one prev that is the very
initial one, then we do the quoting, i.e. the result of expanding
the enclosed string in %(start-something)...%(end) is quoted only
when that appears at the top level, which feels more correct...
As this already allows us to use nested construct, I think we would
want to have test that uses
%(align:30,left)%(align:20,right)%(refname:short)%(end)%(end)
or something like that ;-)
Very nice.
--
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] am: terminate state files with a newline
On Sun, Aug 23, 2015 at 01:50:53PM +0800, Paul Tan wrote: 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?) :-/ . zsh's vcs_info does read files in those directories in order to determine which patches have been applied. I just submitted a patch to zsh that fixed warnings when a conflict occurred with git rebase -m. I expect that unless we provide a programmatic way to discover all of that information trivially (and maybe even then, due to compatibility with older versions of git), people are going to poke around those directories. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH v13 00/12] port tag.c to use ref-filter APIs
Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt index 1997657..06d468e 100644 --- a/Documentation/git-for-each-ref.txt +++ b/Documentation/git-for-each-ref.txt @@ -133,7 +133,8 @@ align:: `position` is either left, right or middle and `width` is the total length of the content with alignment. If the contents length is more than the width then no alignment is -performed. +performed. If used with '--quote' everything in between %(align:..) +and %(end) is quoted. There's no --quote, there are --shell, --python, ... (well, actually, I would have prefered to have a single --quote=language option, but that's not how it is now). I had already commented on a preliminary version of this series off-list. I think all my previous comments have been taken into account. Thanks, both. I think this is pretty close to being satisfactory ;-) There may probably be a handful of minor nits like the above that need to be addressed, but I do not think I saw anything glaringly wrong that makes the series unsalvageable. It was a very pleasant read. It's almost there, and I am very happy to see how this and other series evolved so far ;-) -- 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
git Dockerfile for ppc64le
Hi All, I have built and tested the latest version on git on multiple distributions including Ubuntu 15.04 on the ppc64le architecture. I have run the complete test suite and verified that there are no test failures. I have also written a dockerfile for this for Ubuntu 15.04 and wanted to understand if there is any way I can contribute it to the git community? Any guidance on this would be great help. Thanks! Regards, Priya -- 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
допоможіть сайту нашої школи http://154school.kiev.ua/ - будь ласка, відкрийте його для перегляду однієї-двох сторінок,
Доброго дня, будь ласка, просимо переглянути наш шкільний сайт, якщо це не важко для вас, http://154school.kiev.ua/ - будь ласка, відкрийте його для перегляду однієї-двох сторінок, і на будь-якій сторінці один раз натисніть на рекламний банер, який вам найбільш цікавий, це Ваша допомога, щоб ми могли заплатити за хостинг нашого шкільного сайту, дякуємо, системний адміністратор ad...@154school.kiev.ua http://154school.kiev.ua/ hello, our school site require you view, please, if it's not hard for you, http://154school.kiev.ua/ - please open it for viewing our site - one or two pages, and on any page, click once on the advertising banner that most interesting for you, it is your help to pay for hosting our school site, thank you system Administrator ad...@154school.kiev.ua http://154school.kiev.ua/ -- 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 v5 2/2] worktree: add 'list' command
On Sat, Aug 22, 2015 at 11:51 PM, Michael Rappazzo rappa...@gmail.com wrote: 'git worktree list' uses the for_each_worktree function to iterate, and outputs in the format: 'worktree (short-ref)' Signed-off-by: Michael Rappazzo rappa...@gmail.com --- 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 new-branch] path [branch] 'git worktree prune' [-n] [-v] [--expire 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. Maybe just and the branch or revision currently checked out in that worktree.? -- Mikael Magnusson -- 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: bug: git branch -d and case-insensitive file-systems
On Mon, Aug 24, 2015 at 12:11:13PM -0400, Aaron Dufour wrote:
I use git (2.2.1) on OS X (10.9.5) and recently my repo got into a bad
state. I think this involves a mis-handling of case-insensitive file
systems.
This reproduces the problem:
git init
Initialized empty Git repository in
/Users/aarond_local/code/git-test/.git/
git commit --allow-empty -m 'first commit'
[master (root-commit) 923d8b8] first commit
git checkout -b feature
Switched to a new branch 'feature'
git checkout -b Feature
fatal: A branch named 'Feature' already exists.
git checkout -B Feature
Switched to and reset branch 'Feature'
git branch -d feature
Deleted branch feature (was 923d8b8).
git log
fatal: bad default revision 'HEAD'
I don't work on a case-insensitive filesystem, so my knowledge may be
out of date, but as far as I know, we do not do anything special to
handle ref case-sensitivity. I expect your problem would go away with
this patch:
diff --git a/builtin/branch.c b/builtin/branch.c
index 58aa84f..c5545de 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -19,6 +19,7 @@
#include column.h
#include utf8.h
#include wt-status.h
+#include dir.h
static const char * const builtin_branch_usage[] = {
N_(git branch [options] [-r | -a] [--merged | --no-merged]),
@@ -223,7 +224,7 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
int flags = 0;
strbuf_branchname(bname, argv[i]);
- if (kinds == REF_LOCAL_BRANCH !strcmp(head, bname.buf)) {
+ if (kinds == REF_LOCAL_BRANCH !strcmp_icase(head,
bname.buf)) {
error(_(Cannot delete the branch '%s'
which you are currently on.), bname.buf);
ret = 1;
but I think that is just the tip of the iceberg. E.g. (on a vfat
filesystem I just created):
$ git init
$ git commit -q --allow-empty -m one
$ git branch foo
$ git branch FOO
fatal: A branch named 'FOO' already exists.
$ git pack-refs --all --prune ;# usually run as part of git-gc
$ git commit -q --allow-empty -m two
$ git branch FOO
$ git for-each-ref --format='%(refname) %(subject)'
refs/heads/FOO two
refs/heads/foo one
refs/heads/master two
Now the patch I showed above would do the wrong thing. Running git
checkout foo; git branch -d FOO would be rejected, even though I really
do have two separate branches.
It would be a much more invasive change to fix this correctly. It is
probably less work overall to move to a pluggable ref system, and to
design ref storage that isn't dependent on the filesystem (this work is
already underway).
In the meantime, I think the best advice for mixed-case branch names on
a case-insensitive filesystem is: don't.
-Peff
--
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
