[PATCH v3] send-email: recognize absolute path on Windows
From: Erik Faye-Lund
On Windows, absolute paths might start with a DOS drive prefix,
which these two checks failed to recognize.
Unfortunately, we cannot simply use the file_name_is_absolute
helper in File::Spec::Functions, because Git for Windows has an
MSYS-based Perl, where this helper doesn't grok DOS
drive-prefixes.
So let's manually check for these in that case, and fall back to
the File::Spec-helper on other platforms (e.g Win32 with native
Perl)
Signed-off-by: Erik Faye-Lund
---
So here's a version that does the old and long-time tested
approach without requiring breaking changes to msysGit's perl.
git-send-email.perl | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index fdb0029..8f5f986 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1113,6 +1113,18 @@ sub ssl_verify_params {
}
}
+sub file_name_is_absolute {
+ my ($path) = @_;
+
+ # msys does not grok DOS drive-prefixes
+ if ($^O eq 'msys') {
+ return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
+ }
+
+ require File::Spec::Functions;
+ return File::Spec::Functions::file_name_is_absolute($path);
+}
+
# Returns 1 if the message was sent, and 0 otherwise.
# In actuality, the whole program dies when there
# is an error sending a message.
@@ -1197,7 +1209,7 @@ X-Mailer: git-send-email $gitversion
if ($dry_run) {
# We don't want to send the email.
- } elsif ($smtp_server =~ m#^/#) {
+ } elsif (file_name_is_absolute($smtp_server)) {
my $pid = open my $sm, '|-';
defined $pid or die $!;
if (!$pid) {
@@ -1271,7 +1283,7 @@ X-Mailer: git-send-email $gitversion
printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
} else {
print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
- if ($smtp_server !~ m#^/#) {
+ if (!file_name_is_absolute($smtp_server)) {
print "Server: $smtp_server\n";
print "MAIL FROM:<$raw_from>\n";
foreach my $entry (@recipients) {
--
1.9.0.msysgit.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase: Print name of rev when using shorthand
Thank you for the feedback!
> Imagine the case where there are more than one branches
> whose tip points at the commit you came from.
> name-rev will not be able to pick correctly which one to report.
I see. Yes, you're exactly right; the following demonstrates
the problem:
$ git checkout -b xylophone master
$ git checkout -b aardvark master
$ git name-rev --name-only @{-1} # I'd want "xylophone", but this
outputs "aardvark"
So it appears name-rev is not up to the task here.
> I think you would want to use something like:
>
> upstream_name=$(git rev-parse --symbolic-full-name @{-1})
> if test -n "$upstream"
> then
> upstream_name=${upstream_name#refs/heads/}
> else
> upstream_name="@{-1}"
> fi
>
> if the change is to be made at that point in the code.
I agree, I will re-roll the patch to use this approach.
> I also wonder if "git rebase @{-1}" deserve a similar translation
> like you are giving "git rebase -".
Personally, I've been using the "-" shorthand with "git checkout"
for a year or so, but only learned about "@{-1}" a few months ago.
I think those who use "@{-1}" are familiar enough with the concept
that they don't need to have the reference translated to a symbolic
full name. Users familiar with "-" might not be aware of "@{-1}",
however, so I'd prefer not to output it as we are currently.
Furthermore, were we to translate "@{-1}", does that mean we
should also translate "@{-2}" or prior? I don't think that's the case,
but then only translating "@{-1}" would seem inconsistent.
>From that point of view I'd prefer to simply translate "-",
not "@{-1}".
- Brian Gesiak
--
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] git-rebase: Print name of rev when using shorthand
The output from a successful invocation of the shorthand command
"git rebase -" is something like "Fast-forwarded HEAD to @{-1}",
which includes a relative reference to a revision. Other commands
that use the shorthand "-", such as "git checkout -", typically
display the symbolic name of the revision.
Change rebase to output the symbolic name of the revision when using
the shorthand. For the example above, the new output is
"Fast-forwarded HEAD to master", assuming "@{-1}" is a reference to
"master".
- Use "git rev-parse" to retreive the name of the rev.
- Update the tests in light of this new behavior.
Requested-by: John Keeping
Signed-off-by: Brian Gesiak
---
git-rebase.sh | 8 +++-
t/t3400-rebase.sh | 4 +---
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 2c75e9f..42d34a6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -455,7 +455,13 @@ then
*) upstream_name="$1"
if test "$upstream_name" = "-"
then
- upstream_name="@{-1}"
+ upstream_name=`git rev-parse --symbolic-full-name @{-1}`
+ if test -n "$upstream_name"
+ then
+ upstream_name=${upstream_name#refs/heads/}
+ else
+ upstream_name="@{-1}"
+ fi
fi
shift
;;
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 80e0a95..2b99940 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -91,7 +91,7 @@ test_expect_success 'rebase from ambiguous branch name' '
test_expect_success 'rebase off of the previous branch using "-"' '
git checkout master &&
git checkout HEAD^ &&
- git rebase @{-1} >expect.messages &&
+ git rebase master >expect.messages &&
git merge-base master HEAD >expect.forkpoint &&
git checkout master &&
@@ -100,8 +100,6 @@ test_expect_success 'rebase off of the previous branch
using "-"' '
git merge-base master HEAD >actual.forkpoint &&
test_cmp expect.forkpoint actual.forkpoint &&
- # the next one is dubious---we may want to say "-",
- # instead of @{-1}, in the message
test_i18ncmp expect.messages actual.messages
'
--
1.9.0.259.gc5d75e8.dirty
--
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 4/6] contrib/credential/osxkeychain/git-credential-osxkeychain.c: reduce scope of variables
Signed-off-by: Elia Pinto
---
contrib/credential/osxkeychain/git-credential-osxkeychain.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
index bcd3f57..5ae09f6 100644
--- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
+++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
@@ -163,12 +163,12 @@ static void read_credential(void)
int main(int argc, const char **argv)
{
- const char *usage =
- "usage: git credential-osxkeychain ";
-
if (!argv[1])
+ {
+ const char *usage =
+ "usage: git credential-osxkeychain ";
die(usage);
-
+ }
read_credential();
if (!strcmp(argv[1], "get"))
--
1.7.10.4
--
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/6] compat/regex/regex_internal.c: reduce scope of variables
Signed-off-by: Elia Pinto
---
compat/regex/regex_internal.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/compat/regex/regex_internal.c b/compat/regex/regex_internal.c
index d4121f2..a7a71ec 100644
--- a/compat/regex/regex_internal.c
+++ b/compat/regex/regex_internal.c
@@ -707,7 +707,6 @@ re_string_reconstruct (re_string_t *pstr, int idx, int
eflags)
#ifdef RE_ENABLE_I18N
if (pstr->mb_cur_max > 1)
{
- int wcs_idx;
wint_t wc = WEOF;
if (pstr->is_utf8)
@@ -738,11 +737,11 @@ re_string_reconstruct (re_string_t *pstr, int idx, int
eflags)
mbstate_t cur_state;
wchar_t wc2;
int mlen = raw + pstr->len - p;
- unsigned char buf[6];
size_t mbclen;
if (BE (pstr->trans != NULL, 0))
{
+ unsigned char buf[6];
int i = mlen < 6 ? mlen : 6;
while (--i >= 0)
buf[i] = pstr->trans[p[i]];
@@ -778,6 +777,7 @@ re_string_reconstruct (re_string_t *pstr, int idx, int
eflags)
? CONTEXT_NEWLINE : 0));
if (BE (pstr->valid_len, 0))
{
+ int wcs_idx;
for (wcs_idx = 0; wcs_idx < pstr->valid_len; ++wcs_idx)
pstr->wcs[wcs_idx] = WEOF;
if (pstr->mbs_allocated)
@@ -925,7 +925,6 @@ static unsigned int
internal_function
re_string_context_at (const re_string_t *input, int idx, int eflags)
{
- int c;
if (BE (idx < 0, 0))
/* In this case, we use the value stored in input->tip_context,
since we can't know the character in input->mbs[-1] here. */
@@ -957,6 +956,7 @@ re_string_context_at (const re_string_t *input, int idx,
int eflags)
else
#endif
{
+ int c;
c = re_string_byte_at (input, idx);
if (bitset_contain (input->word_char, c))
return CONTEXT_WORD;
--
1.7.10.4
--
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/6] compat/regex/regcomp.c: reduce scope of variables
Signed-off-by: Elia Pinto
---
compat/regex/regcomp.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/compat/regex/regcomp.c b/compat/regex/regcomp.c
index 06f3088..c7da378 100644
--- a/compat/regex/regcomp.c
+++ b/compat/regex/regcomp.c
@@ -368,7 +368,6 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t
*init_state,
else if (type == COMPLEX_BRACKET)
{
re_charset_t *cset = dfa->nodes[node].opr.mbcset;
- int i;
# ifdef _LIBC
/* See if we have to try all bytes which start multiple collation
@@ -380,6 +379,7 @@ re_compile_fastmap_iter (regex_t *bufp, const re_dfastate_t
*init_state,
if (_NL_CURRENT_WORD (LC_COLLATE, _NL_COLLATE_NRULES) != 0
&& (cset->ncoll_syms || cset->nranges))
{
+ int i;
const int32_t *table = (const int32_t *)
_NL_CURRENT (LC_COLLATE, _NL_COLLATE_TABLEMB);
for (i = 0; i < SBC_MAX; ++i)
@@ -598,7 +598,7 @@ static bitset_t utf8_sb_map;
static void
free_dfa_content (re_dfa_t *dfa)
{
- int i, j;
+ int i;
if (dfa->nodes)
for (i = 0; i < dfa->nodes_len; ++i)
@@ -621,6 +621,7 @@ free_dfa_content (re_dfa_t *dfa)
if (dfa->state_table)
for (i = 0; i <= dfa->state_hash_mask; ++i)
{
+ int j;
struct re_state_table_entry *entry = dfa->state_table + i;
for (j = 0; j < entry->num; ++j)
{
@@ -994,7 +995,7 @@ free_workarea_compile (regex_t *preg)
static reg_errcode_t
create_initial_state (re_dfa_t *dfa)
{
- int first, i;
+ int first;
reg_errcode_t err;
re_node_set init_nodes;
@@ -1011,6 +1012,8 @@ create_initial_state (re_dfa_t *dfa)
Then we add epsilon closures of the nodes which are the next nodes of
the back-references. */
if (dfa->nbackref > 0)
+{
+int i;
for (i = 0; i < init_nodes.nelem; ++i)
{
int node_idx = init_nodes.elems[i];
@@ -1044,6 +1047,7 @@ create_initial_state (re_dfa_t *dfa)
}
}
}
+}
/* It must be the first time to invoke acquire_state. */
dfa->init_state = re_acquire_state_context (&err, dfa, &init_nodes, 0);
@@ -1682,7 +1686,6 @@ static reg_errcode_t
calc_eclosure_iter (re_node_set *new_set, re_dfa_t *dfa, int node, int root)
{
reg_errcode_t err;
- int i;
re_node_set eclosure;
int ret;
int incomplete = 0;
@@ -1708,6 +1711,8 @@ calc_eclosure_iter (re_node_set *new_set, re_dfa_t *dfa,
int node, int root)
/* Expand each epsilon destination nodes. */
if (IS_EPSILON_NODE(dfa->nodes[node].type))
+{
+int i;
for (i = 0; i < dfa->edests[node].nelem; ++i)
{
re_node_set eclosure_elem;
@@ -1741,6 +1746,7 @@ calc_eclosure_iter (re_node_set *new_set, re_dfa_t *dfa,
int node, int root)
re_node_set_free (&eclosure_elem);
}
}
+}
/* An epsilon closure includes itself. */
ret = re_node_set_insert (&eclosure, node);
@@ -2630,7 +2636,6 @@ build_range_exp (bitset_t sbcset, bracket_elem_t
*start_elem,
bracket_elem_t *end_elem)
# endif /* not RE_ENABLE_I18N */
{
- unsigned int start_ch, end_ch;
/* Equivalence Classes and Character Classes can't be a range start/end. */
if (BE (start_elem->type == EQUIV_CLASS || start_elem->type == CHAR_CLASS
|| end_elem->type == EQUIV_CLASS || end_elem->type == CHAR_CLASS,
@@ -2647,6 +2652,7 @@ build_range_exp (bitset_t sbcset, bracket_elem_t
*start_elem,
# ifdef RE_ENABLE_I18N
{
+unsigned int start_ch, end_ch;
wchar_t wc;
wint_t start_wc;
wint_t end_wc;
@@ -2728,6 +2734,7 @@ build_range_exp (bitset_t sbcset, bracket_elem_t
*start_elem,
# else /* not RE_ENABLE_I18N */
{
unsigned int ch;
+unsigned int start_ch, end_ch;
start_ch = ((start_elem->type == SB_CHAR ) ? start_elem->opr.ch
: ((start_elem->type == COLL_SYM) ? start_elem->opr.name[0]
: 0));
--
1.7.10.4
--
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/6] contrib/credential/wincred/git-credential-wincred.c: reduce scope of variables
Signed-off-by: Elia Pinto
---
contrib/credential/wincred/git-credential-wincred.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c
b/contrib/credential/wincred/git-credential-wincred.c
index a1d38f0..edff71c 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -258,11 +258,13 @@ static void read_credential(void)
int main(int argc, char *argv[])
{
- const char *usage =
- "usage: git credential-wincred \n";
if (!argv[1])
+ {
+ const char *usage =
+ "usage: git credential-wincred \n";
die(usage);
+ }
/* git use binary pipes to avoid CRLF-issues */
_setmode(_fileno(stdin), _O_BINARY);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] compat/regex/regexec.c: reduce scope of variables
Signed-off-by: Elia Pinto
---
compat/regex/regexec.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/compat/regex/regexec.c b/compat/regex/regexec.c
index eb5e1d4..f86dbeb 100644
--- a/compat/regex/regexec.c
+++ b/compat/regex/regexec.c
@@ -1254,12 +1254,12 @@ proceed_next_node (const re_match_context_t *mctx, int
nregs, regmatch_t *regs,
struct re_fail_stack_t *fs)
{
const re_dfa_t *const dfa = mctx->dfa;
- int i, err;
+ int err;
if (IS_EPSILON_NODE (dfa->nodes[node].type))
{
re_node_set *cur_nodes = &mctx->state_log[*pidx]->nodes;
re_node_set *edests = &dfa->edests[node];
- int dest_node;
+ int dest_node, i;
err = re_node_set_insert (eps_via_nodes, node);
if (BE (err < 0, 0))
return -2;
@@ -1446,9 +1446,9 @@ set_regs (const regex_t *preg, const re_match_context_t
*mctx, size_t nmatch,
if (idx == pmatch[0].rm_eo && cur_node == mctx->last_node)
{
- int reg_idx;
if (fs)
{
+ int reg_idx;
for (reg_idx = 0; reg_idx < nmatch; ++reg_idx)
if (pmatch[reg_idx].rm_so > -1 && pmatch[reg_idx].rm_eo == -1)
break;
@@ -1818,7 +1818,6 @@ add_epsilon_src_nodes (const re_dfa_t *dfa, re_node_set
*dest_nodes,
const re_node_set *candidates)
{
reg_errcode_t err = REG_NOERROR;
- int i;
re_dfastate_t *state = re_acquire_state (&err, dfa, dest_nodes);
if (BE (err != REG_NOERROR, 0))
@@ -1826,6 +1825,7 @@ add_epsilon_src_nodes (const re_dfa_t *dfa, re_node_set
*dest_nodes,
if (!state->inveclosure.alloc)
{
+ int i;
err = re_node_set_alloc (&state->inveclosure, dest_nodes->nelem);
if (BE (err != REG_NOERROR, 0))
return REG_ESPACE;
@@ -3824,7 +3824,6 @@ check_node_accept_bytes (const re_dfa_t *dfa, int
node_idx,
# ifdef _LIBC
const unsigned char *pin
= ((const unsigned char *) re_string_get_buffer (input) + str_idx);
- int j;
uint32_t nrules;
# endif /* _LIBC */
int match_len = 0;
@@ -3867,6 +3866,7 @@ check_node_accept_bytes (const re_dfa_t *dfa, int
node_idx,
for (i = 0; i < cset->ncoll_syms; ++i)
{
const unsigned char *coll_sym = extra + cset->coll_syms[i];
+ int j;
/* Compare the length of input collating element and
the length of current collating element. */
if (*coll_sym != elem_len)
@@ -4004,13 +4004,14 @@ find_collation_sequence_value (const unsigned char
*mbs, size_t mbs_len)
for (idx = 0; idx < extrasize;)
{
- int mbs_cnt, found = 0;
+ int found = 0;
int32_t elem_mbs_len;
/* Skip the name of collating element name. */
idx = idx + extra[idx] + 1;
elem_mbs_len = extra[idx++];
if (mbs_len == elem_mbs_len)
- {
+ {
+ init mbs_cnt;
for (mbs_cnt = 0; mbs_cnt < elem_mbs_len; ++mbs_cnt)
if (extra[idx + mbs_cnt] != mbs[mbs_cnt])
break;
--
1.7.10.4
--
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 6/6] xdiff/xprepare.c: reduce scope of variables
Signed-off-by: Elia Pinto
---
xdiff/xprepare.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 63a22c6..e0b6987 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -161,8 +161,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf,
long narec, xpparam_
xdlclassifier_t *cf, xdfile_t *xdf) {
unsigned int hbits;
long nrec, hsize, bsize;
- unsigned long hav;
- char const *blk, *cur, *top, *prev;
+ char const *blk, *cur, *prev;
xrecord_t *crec;
xrecord_t **recs, **rrecs;
xrecord_t **rhash;
@@ -193,7 +192,9 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf,
long narec, xpparam_
nrec = 0;
if ((cur = blk = xdl_mmfile_first(mf, &bsize)) != NULL) {
+char const *top;
for (top = blk + bsize; cur < top; ) {
+unsigned long hav;
prev = cur;
hav = xdl_hash_record(&cur, top, xpp->flags);
if (nrec >= narec) {
--
1.7.10.4
--
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
Hello
Helló a nevem Nora kedvem, hogy Önnel a kapcsolatot, miután megy keresztül a profil ezen az oldalon, miközben keresi a kapcsolatot. Én nagyon örülök, hogy a barátod, kérjük, ha nem bánja, azt fogom szeretni, hogy írjon nekem vissza az e- mail címemet. ( johnson.nora31 @ yahoo.com) úgy, hogy én is adok neked a képeket, és mondd meg többet az én oldalamra. Alig várom, hogy olvassa el tőled. Hello my name is Nora i am well pleased to contact you after going through your profile on this site while searching for a relationship. i will be very glad to be your friend, please if you wouldn't mind, i will like you to write me back through my email. ( johnson.nor...@yahoo.com) so that i can give you my pictures and tell you more about my self. I am looking forward to read from you. -- 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 5/6] contrib/credential/wincred/git-credential-wincred.c: reduce scope of variables
On Wed, Apr 16, 2014 at 11:33 AM, Elia Pinto wrote:
> Signed-off-by: Elia Pinto
> ---
> contrib/credential/wincred/git-credential-wincred.c |6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/credential/wincred/git-credential-wincred.c
> b/contrib/credential/wincred/git-credential-wincred.c
> index a1d38f0..edff71c 100644
> --- a/contrib/credential/wincred/git-credential-wincred.c
> +++ b/contrib/credential/wincred/git-credential-wincred.c
> @@ -258,11 +258,13 @@ static void read_credential(void)
>
> int main(int argc, char *argv[])
> {
> - const char *usage =
> - "usage: git credential-wincred \n";
>
> if (!argv[1])
> + {
> + const char *usage =
> + "usage: git credential-wincred \n";
> die(usage);
> + }
>
This is not the indentation/bracket-style we use in this source-file.
--
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 4/6] contrib/credential/osxkeychain/git-credential-osxkeychain.c: reduce scope of variables
On Wed, Apr 16, 2014 at 11:33 AM, Elia Pinto wrote:
> Signed-off-by: Elia Pinto
> ---
> contrib/credential/osxkeychain/git-credential-osxkeychain.c |8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> index bcd3f57..5ae09f6 100644
> --- a/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> +++ b/contrib/credential/osxkeychain/git-credential-osxkeychain.c
> @@ -163,12 +163,12 @@ static void read_credential(void)
>
> int main(int argc, const char **argv)
> {
> - const char *usage =
> - "usage: git credential-osxkeychain ";
> -
> if (!argv[1])
> + {
> + const char *usage =
> + "usage: git credential-osxkeychain ";
> die(usage);
> -
> + }
Again, not our code-style.
--
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 6/6] xdiff/xprepare.c: reduce scope of variables
On Wed, Apr 16, 2014 at 11:33 AM, Elia Pinto wrote:
> Signed-off-by: Elia Pinto
> ---
> xdiff/xprepare.c |5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
> index 63a22c6..e0b6987 100644
> --- a/xdiff/xprepare.c
> +++ b/xdiff/xprepare.c
> @@ -161,8 +161,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t
> *mf, long narec, xpparam_
>xdlclassifier_t *cf, xdfile_t *xdf) {
> unsigned int hbits;
> long nrec, hsize, bsize;
> - unsigned long hav;
> - char const *blk, *cur, *top, *prev;
> + char const *blk, *cur, *prev;
> xrecord_t *crec;
> xrecord_t **recs, **rrecs;
> xrecord_t **rhash;
> @@ -193,7 +192,9 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t
> *mf, long narec, xpparam_
>
> nrec = 0;
> if ((cur = blk = xdl_mmfile_first(mf, &bsize)) != NULL) {
> +char const *top;
> for (top = blk + bsize; cur < top; ) {
> +unsigned long hav;
> prev = cur;
We do not indent with spaces.
--
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] Unicode: update of combining code points
On 16/04/2014 07:48, Torsten Bögershausen wrote: On 15.04.14 21:10, Peter Krefting wrote: Torsten Bögershausen: diff --git a/utf8.c b/utf8.c index a831d50..77c28d4 100644 --- a/utf8.c +++ b/utf8.c Is there a script that generates this code from the Unicode database files, or did you hand-update it? Some of the code points which have "0 length on the display" are called "combining", others are called "vowels" or "accents". E.g. 5BF is not marked any of them, but if you look at the glyph, it should be combining (please correct me if that is wrong). Indeed it is combining (more specifically it has General Category "Nonspacing_Mark" = "Mn"). If I could have found a file which indicates for each code point, what it is, I could write a script. The most complete and machine-readable data are in these files: http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt The general categories can also be seen more legibly in: http://www.unicode.org/Public/UCD/latest/ucd/extracted/DerivedGeneralCategory.txt For docs, see: http://www.unicode.org/reports/tr44/ http://www.unicode.org/reports/tr11/ http://www.unicode.org/ucd/ The existing utf8.c comments describe the attributes being selected from the tables (general categories "Cf","Mn","Me", East Asian Width "W", "F"). And they suggest that the combining character table was originally auto-generated from UnicodeData.txt with a "uniset" tool. Presumably this? https://github.com/depp/uniset The fullwidth-checking code looks like it was done by hand, although apparently uniset can process EastAsianWidth.txt. Kevin -- 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: What's cooking in git.git (Apr 2014, #03; Fri, 11)
On 16/04/14 00:18, Duy Nguyen wrote:
> On Tue, Apr 15, 2014 at 11:45 PM, Junio C Hamano wrote:
>> Ramsay Jones writes:
>>
>>> On 11/04/14 23:22, Junio C Hamano wrote:
>>> [...]
[New Topics]
* nd/index-pack-one-fd-per-thread (2014-04-09) 1 commit
- index-pack: work around thread-unsafe pread()
Enable threaded index-pack on platforms without thread-unsafe
pread() emulation.
Will merge to 'next' and keep it there for the remainder of the cycle.
>>>
>>> The commit message for commit 512ebe5d ("index-pack: work around
>>> thread-unsafe pread()", 25-03-2014) is a little misleading.
>>
>> OK. Can we have a concrete alternative?
>>
>> Multi-threaing of index-pack was disabled with c0f8654
>> (index-pack: Disable threading on cygwin - 2012-06-26), because
>> pread() implementations for Cygwin and MSYS were not thread
>> safe. Recent Cygwin does offer usable pread() and we enabled
>> multi-threading with 103d530f (Cygwin 1.7 has thread-safe pread,
>> 2013-07-19).
>>
>> Work around this problem on platforms with a thread-unsafe
>> pread() emulation by opening one file handle per thread; it
>> would prevent parallel pread() on different file handles from
>> stepping on each other.
>>
>> Also remove NO_THREAD_SAFE_PREAD that was introduced in c0f8654
>> because it's no longer used anywhere.
>>
>> This workaround is unconditional, even for platforms with
>> thread-safe pread() because the overhead is small (a couple file
>> handles more) and not worth fragmenting the code.
>>
>
> OK to me.
Yep, this looks good to me too.
Thanks!
ATB,
Ramsay Jones
--
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: wrong handling of text git attribute leading to files incorrectly reported as modified
Am 15.04.2014 um 23:23 schrieb Junio C Hamano : > Brandon McCaig writes: > >> That is for your benefit, and for easily sharing that configuration >> with collaborators. Git only cares that the file exists in your >> working tree at run-time. > > It is a lot more than "for sharing". If you made .gitignore only > effective after it gets committed, you cannot test your updated > version of .gitignore is correct before committing the change. Ok, I can follow that logic for .gitignore, but I was talking about .gitattributes and I always thought that .gitattributes as belonging to the repository, since it affects a) how files are checked out and b) how they are stored inside the repository. If committing .gitattributes were only for sharing convenience, git couldn’t decide whether to convert line endings when checking out a file. The same behavior doesn’t apply to .gitignore, because git will checkout a file that was added even though it matches an ignore pattern in .gitignore. -- 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: On "interpret-trailers" standalone tool
On Mon, Apr 14, 2014 at 11:41 PM, Junio C Hamano wrote: > Christian Couder writes: > >> Yeah, except that we could add for example a '-o' option that would >> take a directory as argument and that would mean that the command >> should operate on all the files in this directory. It would be like >> the -o option of the format-patch command. > > For output for which users do not know offhand what files are to be > produced, giving a single directory with -o makes tons of sense, but > for input, naming each individual file (and with help with shell > globs *) is a lot more natural UNIX tool way, I would think. Yeah, but the "git interpret-trailers" command is a special, because, if it takes files as arguments, then it is logical that its output would be also files and not stdout. (See also at the end of this message.) > "Take > everything from this directory" cannot be substitute for that, even > though the reverse (i.e. by naming the input files with "dir/*") is > true. It is not a viable replacement. > >> First, if you think that the command might often be used along with >> format-patch, > > ... I am not singling out format-patch output. Any text file/stream > that has the commit log message may benefit from the "trailers" filter, > and format-patch output is merely one very obvious example. As to > the detection of the end of commit log message, the current "EOF is > where the log message ends (but we would remote trailing blank line)" > can easily be updated to "EOF or the first three-dash line". Ok, I think that it's an interesting feature anyway, so I can add it now instead of later. >> Third, if trailers arguments are passed to the command using an >> option like "-z token=value" or "-z token:value", it would be nice >> to the user for consistency if the same option could be used when >> passing the same arguments to "git commit" and perhaps other >> commands like "git rebase", "git cherry-pick" and so on. This >> means that we now have to choose carefully the name of this >> option. Perhaps we can just give it a long name now like --trailer >> and care later about a short name,... > > Absolutely. That is a very sensible way to go. Ok, I will use "--trailer" then. As I said in my previous message, this unfortunately means that the command will not be very user friendly until we integrate it with other commands like "git commit" and find a short option name that hopefully work for all the commands. >> Fourth, some users might want the command to be passed some files as >> input, but they might not want the command to modify these input >> files. They might prefer the command to write its ouput into another >> set of output files. Maybe a syntax like cat or sed is not very well >> suited for this kind of use, while having a -o option for the output >> directory and a -i option for the input directory (if different from >> the output dir) would be nicer. > > Sure. I would expect we would require something like Perl's '-i' > (in-place rewrite) option for this sequence to really work: > > git format-patch -o there -5 > git that-command --options -i there/* > > and without, I would expect the output to come to its standard > output. If the input comes from stdin, then I agree that the command should be a filter, so its output should be on stdout. But if the input comes from files given as arguments, then I would say that the command should behave as an editor and by default it should edit its input file inplace. Its input and output files should be different only if it is given one or more special option, Otherwise the example you gave: $ git format-patch -5 --cover-letter -o +my-series/ my-topic $ git interpret-trailers "some args" ./+my-series/0*.patch would result in having on stdout all the patches edited by "git interpret-trailers". How would people could then easily send these edited patches? Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] config.c: fix a compiler warning
Date: Thu, 10 Apr 2014 16:37:15 +0200
This change fixes a gcc warning when building msysGit.
---
config.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 314d8ee..0b7e4f8 100644
--- a/config.c
+++ b/config.c
@@ -572,7 +572,7 @@ static void die_bad_number(const char *name, const char
*value)
int git_config_int(const char *name, const char *value)
{
- int ret;
+ int ret = 0;
if (!git_parse_int(value, &ret))
die_bad_number(name, value);
return ret;
@@ -580,7 +580,7 @@ int git_config_int(const char *name, const char *value)
int64_t git_config_int64(const char *name, const char *value)
{
- int64_t ret;
+ int64_t ret = 0;
if (!git_parse_int64(value, &ret))
die_bad_number(name, value);
return ret;
--
1.9.2.msysgit.0.154.g978f18d
--
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] git tag --contains : avoid stack overflow
From: Jean-Jacques Lafay
Date: Sat, 10 Nov 2012 18:36:10 +0100
In large repos, the recursion implementation of contains(commit,
commit_list) may result in a stack overflow. Replace the recursion with
a loop to fix it.
This problem is more apparent on Windows than on Linux, where the stack
is more limited by default.
See also this thread on the msysGit list:
https://groups.google.com/d/topic/msysgit/FqT6boJrb2g/discussion
[jes: re-written to imitate the original recursion more closely]
Thomas Braun pointed out several documentation shortcomings.
Signed-off-by: Jean-Jacques Lafay
Signed-off-by: Johannes Schindelin
Tested-by: Stepan Kasal
Thanks-to: Thomas Braun
---
builtin/tag.c | 81 --
t/t7004-tag.sh | 21 +++
2 files changed, 88 insertions(+), 14 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 74d3780..79c8c28 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -73,11 +73,13 @@ static int in_commit_list(const struct commit_list *want,
struct commit *c)
return 0;
}
-static int contains_recurse(struct commit *candidate,
+/*
+ * Test whether the candidate or one of its parents is contained in the list.
+ * Do not recurse to find out, though, but return -1 if inconclusive.
+ */
+static int contains_test(struct commit *candidate,
const struct commit_list *want)
{
- struct commit_list *p;
-
/* was it previously marked as containing a want commit? */
if (candidate->object.flags & TMP_MARK)
return 1;
@@ -85,26 +87,77 @@ static int contains_recurse(struct commit *candidate,
if (candidate->object.flags & UNINTERESTING)
return 0;
/* or are we it? */
- if (in_commit_list(want, candidate))
+ if (in_commit_list(want, candidate)) {
+ candidate->object.flags |= TMP_MARK;
return 1;
+ }
if (parse_commit(candidate) < 0)
return 0;
- /* Otherwise recurse and mark ourselves for future traversals. */
- for (p = candidate->parents; p; p = p->next) {
- if (contains_recurse(p->item, want)) {
- candidate->object.flags |= TMP_MARK;
- return 1;
- }
- }
- candidate->object.flags |= UNINTERESTING;
- return 0;
+ return -1;
+}
+
+/*
+ * Mimicking the real stack, this stack lives on the heap, avoiding stack
+ * overflows.
+ *
+ * At each recursion step, the stack items points to the commits whose
+ * ancestors are to be inspected.
+ */
+struct stack {
+ int nr, alloc;
+ struct stack_entry {
+ struct commit *commit;
+ struct commit_list *parents;
+ } *stack;
+};
+
+static void push_to_stack(struct commit *candidate, struct stack *stack)
+{
+ int index = stack->nr++;
+ ALLOC_GROW(stack->stack, stack->nr, stack->alloc);
+ stack->stack[index].commit = candidate;
+ stack->stack[index].parents = candidate->parents;
}
static int contains(struct commit *candidate, const struct commit_list *want)
{
- return contains_recurse(candidate, want);
+ struct stack stack = { 0, 0, NULL };
+ int result = contains_test(candidate, want);
+
+ if (result >= 0)
+ return result;
+
+ push_to_stack(candidate, &stack);
+ while (stack.nr) {
+ struct stack_entry *entry = &stack.stack[stack.nr - 1];
+ struct commit *commit = entry->commit;
+ struct commit_list *parents = entry->parents;
+
+ if (!parents) {
+ commit->object.flags = UNINTERESTING;
+ stack.nr--;
+ }
+ /*
+* If we just popped the stack, parents->item has been marked,
+* therefore contains_test will return a meaningful 0 or 1.
+*/
+ else switch (contains_test(parents->item, want)) {
+ case 1:
+ commit->object.flags |= TMP_MARK;
+ stack.nr--;
+ break;
+ case 0:
+ entry->parents = parents->next;
+ break;
+ default:
+ push_to_stack(parents->item, &stack);
+ break;
+ }
+ }
+ free(stack.stack);
+ return contains_test(candidate, want);
}
static void show_tag_lines(const unsigned char *sha1, int lines)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index c8d6e9f..edaff13 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1380,4 +1380,25 @@ test_expect_success 'multiple --points-at are OR-ed
together' '
test_cmp expect actual
'
+>expect
+# ulimit is a bash builtin; we can rely on that in MinGW, but nowhere else
+test_expect_success MINGW '--contains works in a deep repo' '
+
[PATCH] Update SVN.pm
From: RomanBelinsky
Date: Tue, 11 Feb 2014 18:23:02 +0200
fix parsing error for dates like:
2014-01-07T5:58:36.048176Z
previous regex can parse only:
2014-01-07T05:58:36.048176Z
reproduced in my svn repository during conversion.
Signed-off-by: Stepan Kasal
---
perl/Git/SVN.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index a59564f..09cff13 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1321,7 +1321,7 @@ sub get_untracked {
sub parse_svn_date {
my $date = shift || return '+ 1970-01-01 00:00:00';
my ($Y,$m,$d,$H,$M,$S) = ($date =~ /^(\d{4})\-(\d\d)\-(\d\d)T
- (\d\d)\:(\d\d)\:(\d\d)\.\d*Z$/x) or
+ (\d\d?)\:(\d\d)\:(\d\d)\.\d*Z$/x) or
croak "Unable to parse date: $date\n";
my $parsed_date;# Set next.
--
1.9.2.msysgit.0.154.g978f18d
--
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 16/25] commit_lock_file(): die() if called for unlocked lockfile object
On 04/15/2014 08:49 AM, Johannes Sixt wrote:
> Am 4/14/2014 15:54, schrieb Michael Haggerty:
>> diff --git a/lockfile.c b/lockfile.c
>> index 664b0c3..1453a7a 100644
>> --- a/lockfile.c
>> +++ b/lockfile.c
>> @@ -292,6 +292,9 @@ int commit_lock_file(struct lock_file *lk)
>> if (lk->fd >= 0 && close_lock_file(lk))
>> return -1;
>>
>> +if (!lk->filename[0])
>> +die("BUG: attempt to commit unlocked object");
>> +
>
> Shouldn't this be the first thing to check after entering the function?
You're right; I will change it.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] config.c: fix a compiler warning
On Wed, Apr 16, 2014 at 04:13:53PM +0200, Stepan Kasal wrote:
> Date: Thu, 10 Apr 2014 16:37:15 +0200
>
> This change fixes a gcc warning when building msysGit.
What warning? I'm assuming -Wuninitialized?
> diff --git a/config.c b/config.c
> index 314d8ee..0b7e4f8 100644
> --- a/config.c
> +++ b/config.c
> @@ -572,7 +572,7 @@ static void die_bad_number(const char *name, const char
> *value)
>
> int git_config_int(const char *name, const char *value)
> {
> - int ret;
> + int ret = 0;
> if (!git_parse_int(value, &ret))
> die_bad_number(name, value);
> return ret;
Hmph. Generally gcc should assume that a variable is initialized after a
pointer to it is passed into a function. Unless it inlines that function
and can see that it isn't. But if we do inline git_parse_int, we see
that it "ret" is always initialized if it returns non-zero.
If it also inlines die_bad_number, it would see that we end in die(),
which is marked NORETURN. But if it does not, it will not realize that
we do not get to "return ret" in that case.
So perhaps a better solution is:
diff --git a/config.c b/config.c
index 6821cef..a30cb5c 100644
--- a/config.c
+++ b/config.c
@@ -557,6 +557,7 @@ int git_parse_ulong(const char *value, unsigned long *ret)
return 1;
}
+NORETURN
static void die_bad_number(const char *name, const char *value)
{
const char *reason = errno == ERANGE ?
Does that also silence the warning?
-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 18/25] struct lock_file: declare some fields volatile
On 04/15/2014 08:55 AM, Johannes Sixt wrote: > Am 4/14/2014 15:54, schrieb Michael Haggerty: >> The function remove_lock_file_on_signal() is used as a signal handler. >> It is not realistic to make the signal handler conform strictly to the >> C standard, which is very restrictive about what a signal handler is >> allowed to do. But let's increase the likelihood that it will work: >> >> The lock_file_list global variable and several fields from struct >> lock_file are used by the signal handler. Declare those values >> "volatile" to increase the chance that the signal handler will see a >> valid object state. > > Yes, it's important that the signal handler sees a valid object state, and > "volatile" can help here. But I think the reason why it helps is not > obvious, and it should be mentioned in the commit message: > > It is not so much that "volatile" forces the compiler to lay down each > access of the variable coded in C in the assembly code, but more > importantly, that "volatile" disallows any re-ordering of these accesses. > Then: > > - 'lock->active = 1' must be the last assignment during setup > > - 'lock->active = 0' must be the first assignment during tear-down. > > - Ideally, all members of struct lock_file should be "volatile". > > The last point is important because the compiler is allowed to re-order > accesses to non-"volatile" variables across "volatile" accesses. I say > "ideally" because if filename were defined "volatile filename[PATH_MAX]", > strcpy() could not be used anymore. OTOH, it is unlikely that a compiler > re-orders a strcpy() call across other expressions, and we can get away > without "volatile" in the "filename" case in practice. Thanks for the clarification. I will edit the commit message to better explain the rationale. >> Suggested-by: Johannes Sixt > > Not a big deal, but just in case you re-roll again and you do not forget: > > Johannes Sixt > > is preferred. ACK. Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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] git tag --contains : avoid stack overflow
On Wed, Apr 16, 2014 at 04:15:19PM +0200, Stepan Kasal wrote:
> From: Jean-Jacques Lafay
> Date: Sat, 10 Nov 2012 18:36:10 +0100
>
> In large repos, the recursion implementation of contains(commit,
> commit_list) may result in a stack overflow. Replace the recursion with
> a loop to fix it.
>
> This problem is more apparent on Windows than on Linux, where the stack
> is more limited by default.
I think this is a good thing to be doing, and it looks mostly good to
me. A few comments:
> -static int contains_recurse(struct commit *candidate,
> +/*
> + * Test whether the candidate or one of its parents is contained in the list.
> + * Do not recurse to find out, though, but return -1 if inconclusive.
> + */
> +static int contains_test(struct commit *candidate,
> const struct commit_list *want)
Can we turn this return value into
enum {
CONTAINS_UNKNOWN = -1,
CONTAINS_NO = 0,
CONTAINS_YES = 1,
} contains_result;
to make the code a little more self-documenting?
> static int contains(struct commit *candidate, const struct commit_list *want)
> {
> - return contains_recurse(candidate, want);
> + struct stack stack = { 0, 0, NULL };
> + int result = contains_test(candidate, want);
> +
> + if (result >= 0)
> + return result;
Then this can become:
if (result != CONTAINS_UNKNOWN)
return result;
> + if (!parents) {
> + commit->object.flags = UNINTERESTING;
> + stack.nr--;
> + }
Shouldn't this be "|=" when setting the flag?
> + /*
> + * If we just popped the stack, parents->item has been marked,
> + * therefore contains_test will return a meaningful 0 or 1.
> + */
> + else switch (contains_test(parents->item, want)) {
> + case 1:
> + commit->object.flags |= TMP_MARK;
> + stack.nr--;
> + break;
> + case 0:
> + entry->parents = parents->next;
> + break;
> + default:
> + push_to_stack(parents->item, &stack);
> + break;
> + }
And if we have an enum, this switch() becomes more readable (the
"default" here threw me off initially, because it is actually just
looking for "-1").
> +>expect
> +# ulimit is a bash builtin; we can rely on that in MinGW, but nowhere else
> +test_expect_success MINGW '--contains works in a deep repo' '
> + ulimit -s 64
It would be nice to test this on Linux.
Can we do something like:
test_lazy_prereq BASH 'bash --version'
test_expect_success BASH '--contains works in a deep repo' '
... setup repo ...
bash -c "ulimit -s 64 && git tag --contains HEAD" >actual &&
test_cmp expect actual
'
As a bonus, then our "ulimit" call does not pollute the environment of
subsequent tests.
-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] config.c: fix a compiler warning
On 2014-04-16 17.29, Jeff King wrote:
> On Wed, Apr 16, 2014 at 04:13:53PM +0200, Stepan Kasal wrote:
>
>> Date: Thu, 10 Apr 2014 16:37:15 +0200
>>
>> This change fixes a gcc warning when building msysGit.
[]
> +NORETURN
> static void die_bad_number(const char *name, const char *value)
> {
> const char *reason = errno == ERANGE ?
>
> Does that also silence the warning?
>
> -Peff
This works under gcc 4.2.1 Mac OS: the warning is away.
--
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 1/3] rebase: avoid non-function use of "return" on FreeBSD
"Kyle J. McKay" writes:
> On Apr 14, 2014, at 15:51, Junio C Hamano wrote:
>> I think we would want to see the actual change formatted this way
>> (without needing to pass "-w" to "git show"), as it will make it
>> clear that this artificial extra level of "define the whole thing
>> inside a function and then make a single call to it" is a workaround
>> of specific shell's glitch that we would not have to have in an
>> ideal world ;-)
>>
>> Besides that would make it less likely to cause conflicts with the
>> real changes in flight.
>
> Sounds good to me.
>
>> Please double check what I queued on 'pu' when I push out today's
>> integration result.
>
>> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
>> index a4f683a5..2ab514ea 100644
>> --- a/git-rebase--am.sh
>> +++ b/git-rebase--am.sh
>> @@ -4,6 +4,13 @@
>> # Copyright (c) 2010 Junio C Hamano.
>> #
>>
>> +# The whole contents of the file is run by dot-sourcing this file
>> from
>> +# inside a shell function, and "return"s we see below are expected to
>> +# return from that function that dot-sources us. However, FreeBSD
>> +# /bin/sh misbehaves on such a construct, so we will work it around
>> by
>> +# enclosing the whole thing inside an extra layer of a function.
>> +git_rebase__am () {
>
> I think the wording may be just a bit off:
>
>> and "return"s we see below are expected to return from that function
>> that dot-sources us.
>
> I thought that was one of the buggy behaviors we are working around?
Yeah, it is written as if every reader has the knowledge that the
extra
extra__func () {
...
}
extra__func
around did not originally exist. The description does not read well
with the work-around already there.
> Maybe I'm just reading it wrong...
>
> Does this wording seem any clearer?
>
> and "return"s we see below are expected not to return
> from the function that dot-sources us, but rather to
> the next command after the one in the function that
> dot-sources us.
Not really. The comment was not about explaining "return"s. When a
reader reads the text with the work-around, it is clear that these
"return"s are inside this extra function and there is no possible
interpretation other than that they are to return from the extra
function.
The comment was meant to explain why a seemingly strange "define a
function and then immediately call it just once" pattern is used,
and "Earlier, these returns were not inside any function when this
file is viewed standalone. Because this file is to be dot-sourced
within a function, they were to return from that dot-sourcing
function --- but some shells do not behave that way, hence this
strange construct." was meant to be that explanation, but apparently
it failed to convey what I meant to say.
> If I'm the only one getting a wrong meaning from the comments, then no
> reason to change them.
I agree that the description does not read well with the work-around
already there. I am not sure what would be a better way to phrase
it, though.
--
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 1/6] compat/regex/regcomp.c: reduce scope of variables
Hi, Elia Pinto wrote: > [Subject: compat/regex/regcomp.c: reduce scope of variables] gnulib regex is still maintained upstream and available under the LGPL 2.1+. Shouldn't we make the change there and reimport to make it easier to pull in later changes? Thanks, Jonathan -- 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: wrong handling of text git attribute leading to files incorrectly reported as modified
Frank Ammeter writes: > Am 15.04.2014 um 23:23 schrieb Junio C Hamano : > >> Brandon McCaig writes: >> >>> That is for your benefit, and for easily sharing that configuration >>> with collaborators. Git only cares that the file exists in your >>> working tree at run-time. >> >> It is a lot more than "for sharing". If you made .gitignore only >> effective after it gets committed, you cannot test your updated >> version of .gitignore is correct before committing the change. > > Ok, I can follow that logic for .gitignore, but I was talking about > .gitattributes... They are conceptually the same thing, so if you can follow the logic for .gitignore, you already can follow the logic for .gitattributes. The only two readons we have a separate .gitignore are because other SCMs had a similar mechanism, and because it came before attributes. If we didn't have these two constraints, it would have made a lot more sense to express "this path is to be ignored" by setting "ignored" attribute. -- 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] config.c: mark die_bad_number as NORETURN
On Wed, Apr 16, 2014 at 06:38:19PM +0200, Torsten Bögershausen wrote:
> > Does that also silence the warning?
> >
> This works under gcc 4.2.1 Mac OS: the warning is away.
Thanks. I couldn't test myself, as I could not get gcc to generate the
warning in the first place, but I do not have anything as old as 4.2 on
hand.
Here it is with a commit message.
-- >8 --
Subject: config.c: mark die_bad_number as NORETURN
This can help avoid -Wuninitialized false positives in
git_config_int and git_config_ulong, as the compiler now
knows that we do not return "ret" if we hit the error
codepath.
Signed-off-by: Jeff King
---
config.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/config.c b/config.c
index 6821cef..a30cb5c 100644
--- a/config.c
+++ b/config.c
@@ -557,6 +557,7 @@ int git_parse_ulong(const char *value, unsigned long *ret)
return 1;
}
+NORETURN
static void die_bad_number(const char *name, const char *value)
{
const char *reason = errno == ERANGE ?
--
1.9.1.656.ge8a0637
--
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: wrong handling of text git attribute leading to files incorrectly reported as modified
Am 11.04.2014 22:20, schrieb Frank Ammeter: #!/bin/bash # creating a git repo "repo" rm -rf repo mkdir repo cd repo git init # committing gitattributes with text attribute set for all files echo "* text" > .gitattributes git add .gitattributes git commit -m "added .gitattributes" # add a file with CRLF line ending with text attribute unset echo -e "crlf\r" > crlffile echo "* -text" > .gitattributes git add crlffile git commit -m "added crlffile" git checkout .gitattributes # now "crlffile" shows as modified, even though it isn't. It is. In the repository is stored a crlffile with \r in it which would be changed when you would do a commit (with your current gitattributes) # only way to resolve is to modify .gitattributes No. This works too: git add crlffile git commit -m .# practically removes the \r inside the repository git status crlffile #shows up clean -- 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] git-rebase: Print name of rev when using shorthand
Brian Gesiak writes:
> Personally, I've been using the "-" shorthand with "git checkout"
> for a year or so, but only learned about "@{-1}" a few months ago.
> I think those who use "@{-1}" are familiar enough with the concept
> that they don't need to have the reference translated to a
> symbolic full name. Users familiar with "-" might not be aware of
> "@{-1}", however, so I'd prefer not to output it as we are
> currently.
I do not understand that reasoning.
The concept of "n-th prior checkout" (aka @{-n}) and "immediately
previous checkout" (aka "-") are equivalent, even though the former
may be more generic.
You seem to be saying that those who understand the former are with
superiour mental capacity in general than those who only know the
latter, and they can always remember where they came from. It
sounds similar to an absurd claim (pulled out of thin-air only for
illustration purposes) that French-speaking people are of superiour
mind and do not need as much help with math as English speakers.
> Furthermore, were we to translate "@{-1}", does that mean we
> should also translate "@{-2}" or prior?
Surely, why not. If a user is so forgetful to need help remembering
where s/he was immediately before, wouldn't it be more helpful to
give "here is where you were" reminder for older ones to allow them
to double check they specified the right thing and spot possible
mistakes?
I can buy "that would be a lot more work, and I do not want to do it
(or I do not think I can solve it in a more general way)", though.
--
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] send-email: recognize absolute path on Windows
Erik Faye-Lund writes:
> So let's manually check for these in that case, and fall back to
> the File::Spec-helper on other platforms (e.g Win32 with native
> Perl)
> ...
> +sub file_name_is_absolute {
> + my ($path) = @_;
> +
> + # msys does not grok DOS drive-prefixes
> + if ($^O eq 'msys') {
> + return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
Shouldn't the latter also be anchored at the beginning of the string
with a leading "^"?
> + }
> +
> + require File::Spec::Functions;
> + return File::Spec::Functions::file_name_is_absolute($path);
We already "use File::Spec qw(something else)" at the beginning, no?
Why not throw file_name_is_absolute into that qw() instead?
> +}
> +
> # Returns 1 if the message was sent, and 0 otherwise.
> # In actuality, the whole program dies when there
> # is an error sending a message.
> @@ -1197,7 +1209,7 @@ X-Mailer: git-send-email $gitversion
>
> if ($dry_run) {
> # We don't want to send the email.
> - } elsif ($smtp_server =~ m#^/#) {
> + } elsif (file_name_is_absolute($smtp_server)) {
> my $pid = open my $sm, '|-';
> defined $pid or die $!;
> if (!$pid) {
> @@ -1271,7 +1283,7 @@ X-Mailer: git-send-email $gitversion
> printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
> } else {
> print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
> - if ($smtp_server !~ m#^/#) {
> + if (!file_name_is_absolute($smtp_server)) {
> print "Server: $smtp_server\n";
> print "MAIL FROM:<$raw_from>\n";
> foreach my $entry (@recipients) {
> --
> 1.9.0.msysgit.0
>
> --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] Make update refs more atomic
On Tue, Apr 15, 2014 at 1:32 PM, Michael Haggerty wrote:
> On 04/15/2014 06:33 PM, Ronnie Sahlberg wrote:
>> On Mon, Apr 14, 2014 at 11:36 PM, Michael Haggerty
>> wrote:
>>> [...]
>>> I wonder, however, whether your approach of changing callers from
>>>
>>> lock = lock_ref_sha1_basic() (or varient of)
>>> write_ref_sha1(lock)
>>>
>>> to
>>>
>>> lock = lock_ref_sha1_basic() (or varient of)
>>> write_ref_sha1(lock)
>>> unlock_ref(lock) | commit_ref_lock(lock)
>>>
>>> is not doing work that we will soon need to rework. Would it be jumping
>>> the gun to change the callers to
>>>
>>> transaction = ref_transaction_begin();
>>> ref_transaction_{update,delete,etc}(transaction, ...);
>>> ref_transaction_{commit,rollback}(transaction, ...);
>>>
>>> instead? Then we could bury the details of calling write_ref_sha1() and
>>> commit_lock_ref() inside ref_transaction_commit() rather than having to
>>> expose them in the public API.
>>
>> I think you are right.
>>
>> Lets put this patch series on the backburner for now and start by
>> making all callers use transactions
>> and remove write_ref_sha1() from the public API thar refs.c exports.
>>
>> Once everything is switched over to transactions I can rework this
>> patchseries for ref_transaction_commit()
>> and resubmit to the mailing list.
>
> Sounds good. Rewriting callers to use transactions would be a great
> next step. Please especially keep track of what new features the
> transactions API still needs. More flexible error handling? The
> ability to have steps in the transaction that are "best-effort" (i.e.,
> don't abort the transaction if they fail)? Different reflog messages
> for different updates within the same transaction rather than one reflog
> message for all updates? Etc.
>
> And some callers who currently change multiple references one at a time
> might be able to be rewritten to update the references in a single
> transaction.
As an experiment I rewrite most of the callers to use transactions yesterday.
Most callers would translate just fine, but some callers, such as walker_fetch()
does not yet fit well with the current transaction code.
For example that code does want to first take out locks on all refs
before it does a
lot of processing, with the locks held, before it writes and updates the refs.
Some of my thoughts after going over the callers :
Currently any locking of refs in a transaction only happens during the commit
phase. I think it would be useful to have a mechanism where you could
optionally take out locks for the involved refs early during the transaction.
So that simple callers could continue using
ref_transaction_begin()
ref_transaction_create|update|delete()*
ref_transaction_commit()
but, if a caller such as walker_fetch() could opt to do
ref_transaction_begin()
ref_transaction_lock_ref()*
...do stuff...
ref_transaction_create|update|delete()*
ref_transaction_commit()
In this second case ref_transaction_commit() would only take out any locks that
are missing during the 'lock the refs" loop.
Suggestion 1: Add a ref_transaction_lock_ref() to allow locking a ref
early during
a transaction.
A second idea is to change the signatures for
ref_transaction_create|update|delete()
slightly and allow them to return errors early.
We can check for things like add_update() failing, check that the
ref-name looks sane,
check some of the flags, like if has_old==true then old sha1 should
not be NULL or 0{40}, etc.
Additionally for robustness, if any of these functions detect an error
we can flag this in the
transaction structure and take action during ref_transaction_commit().
I.e. if a ref_transaction_update had a hard failure, do not allow
ref_transaction_commit()
to succeed.
Suggestion 2: Change ref_transaction_create|delete|update() to return an int.
All callers that use these functions should check the function for error.
Suggestion 3: remove the qsort and check for duplicates in
ref_transaction_commit()
Since we are already taking out a lock for each ref we are updating
during the transaction
any duplicate refs will fail the second attempt to lock the same ref which will
implicitly make sure that a transaction will not change the same ref twice.
There are only two caveats I see with this third suggestion.
1, The second lock attempt will always cause a die() since we
eventually would end up
in lock_ref_sha1_basic() and this function will always unconditionally
die() if the lock failed.
But your locking changes are addressing this, right?
(all callers to lock_ref_sha1() or lock_any_ref_for_update() do check
for and handle if the lock
failed, so that change to not die() should be safe)
2, We would need to take care when a lock fails here to print the
proper error message
so that we still show "Multiple updates for ref '%s' not allowed." if
the lock failed because
the transaction had already locked this file.
>
>> Lets start preparing patches to change all external callers to use
>> tran
[PATCH 001/14] howto-index.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
Documentation/howto-index.sh | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/howto-index.sh b/Documentation/howto-index.sh
index a234086..167b363 100755
--- a/Documentation/howto-index.sh
+++ b/Documentation/howto-index.sh
@@ -11,8 +11,8 @@ EOF
for txt
do
- title=`expr "$txt" : '.*/\(.*\)\.txt$'`
- from=`sed -ne '
+ title=$(expr "$txt" : '.*/\(.*\)\.txt$')
+ from=$(sed -ne '
/^$/q
/^From:[]/{
s///
@@ -21,9 +21,9 @@ do
s/^/by /
p
}
- ' "$txt"`
+ ' "$txt")
- abstract=`sed -ne '
+ abstract=$(sed -ne '
/^Abstract:[]/{
s/^[^ ]*//
x
@@ -39,11 +39,11 @@ do
x
p
q
- }' "$txt"`
+ }' "$txt")
if grep 'Content-type: text/asciidoc' >/dev/null $txt
then
- file=`expr "$txt" : '\(.*\)\.txt$'`.html
+ file=$(expr "$txt" : '\(.*\)\.txt$').html
else
file="$txt"
fi
--
1.7.10.4
--
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: [RFC] t7410: 210 tests for various 'git submodule update' scenarios
On Wed, Apr 16, 2014 at 02:54:48AM +0200, Johan Herland wrote:
> This is a work-in-progress to flesh out (and promote discussion about)
> the expected behaviors for all possible scenarios in which
> 'git submodule update' might be run.
This is lovely :).
> +# - current state of submodule:
> +# ?.?.?.1 - not yet cloned
> +# ?.?.?.2 - cloned, detached, HEAD == gitlink
> +# ?.?.?.3 - cloned, detached, HEAD != gitlink
> +# ?.?.?.4 - cloned, on branch foo (exists upstream), HEAD == gitlink
> +# ?.?.?.5 - cloned, on branch foo (exists upstream), HEAD != gitlink
> +# ?.?.?.6 - cloned, on branch bar (MISSING upstream), HEAD == gitlink
> +# ?.?.?.7 - cloned, on branch bar (MISSING upstream), HEAD != gitlink
The remote branches should only matter for the initial clone and
--remote updates. Also, only the configured submodule..branch
(your first axis) should be checked; the locally checked-out submodule
branch shouldn't matter.
> +# T2: Test with submodule..url != submodule's remote.origin.url. Does
> +# "submodule update --remote" sync with submodule..url, or with the
> +# submodule's origin? (or with the submodule's current branch's
> upstream)?
All fetches should currently use the submodule's remote.origin.url.
submodule..url is only used for the initial clone (*.*.*.1), and
never referenced again. This would change using my tightly-bound
submodule proposal [1], where a difference between
submodule..url and the submodule's @{upstream} URL would be
trigger a dirty-tree condition (for folks with tight-bind syncing
enabled) from which you couldn't update before resolving the
difference.
> +# D1: When submodule is already at right commit, checkout-mode currently does
> +# nothing. Should it instead detach, even when no update is needed?
> +# Affects: 1.2.1.4, 1.2.1.6, 2.2.1.4, 2.2.1.6, 3.2.1.4, 3.2.1.6
“Checkout updates always leave a detached HEAD” seems easier to
explain, so I'm leaning that way.
> +# D2: Should all/some of 1.3.*/1.4.* abort/error because we don't know what
> to
> +# merge/rebase with (because .branch is unset)? Or is the current default
> +# to origin/HEAD OK?
> +# Affects: 1.3.*, 1.4.*
Maybe you mean 1.3.*, 1.4.*, and 1.5.* (merge, rebase, and !command)?
In all of these cases, we're integrating the current HEAD with the
gitlinked (*.*.1.*) or remote-tracking branch (*.*.2.*). Since
submodule..branch defaults to master (and may be changed to HEAD
after a long transition period? [2,3]), I don't think we should
abort/error in those cases.
> +# D3: When submodule is already at right commit, merge/rebase-mode currently
> +# does nothing. Should it do something else (e.g. not leave submodule
> +# detached, or checked out on the "wrong" branch (i.e. != .branch))?
> +# (This discussion point is related to D1, D5 and D6)
“Non-checkout updates always leave you on a branch” seems easier to
explain, but I think we'd want to distinguish between the local branch
and the remote submodule..branch [1]. Lacking that distinction,
I'd prefer to leave the checked-out branch unchanged.
> +# D4: When 'submodule update' performs a clone to populate a submodule, it
> +# currently also creates a default branch (named after origin/HEAD) in
> +# that submodule, EVEN WHEN THAT BRANCH WILL NEVER BE USED (e.g. because
> +# we're in checkout-mode and submodule will always be detached). Is this
> +# good, or should the clone performed by 'submodule update' skip the
> +# automatic local branch creation?
> +# Affects: 1.2.*.1, 1.3.*.1, 1.4.*.1, 1.5.*.1,
> +# 2.2.*.1, 2.3.*.1, 2.4.*.1, 2.5.*.1,
> +# 3.2.1.1, 3.3.1.1, 3.4.1.1, 3.5.1.1
“Checkout updates always leave a detached HEAD” seems easier to
explain, so I'm leaning that way.
> +# D5: When in merge/rebase-mode, and 'submodule update' actually ends up
> doing
> +# a merge/rebase, that will happen on the current branch (or detached
> HEAD)
> +# and NOT on the configured (or default) .branch. Is this OK? Should we
> +# abort (or at least warn) instead? (In general, .branch seems only to
> +# affect the submodule's HEAD when the submodule is first cloned.)
> +# (This discussion point is related to D3 and D6)
> +# Affects: 1.3.1.3, 1.3.1.5, 1.3.1.7, 1.3.2.>=2,
> +# 1.4.1.3, 1.4.1.5, 1.4.1.7, 1.4.2.>=2,
> +# 2.3.1.3, 2.3.1.5, 2.3.1.7, 2.3.2.2, 2.3.2.4, 2.3.2.6,
> +# 2.4.1.3, 2.4.1.5, 2.4.1.7, 2.4.2.2, 2.4.2.4, 2.4.2.6
> +# 3.3.1.3, 3.3.1.5, 3.3.1.7
> +# 3.4.1.3, 3.4.1.5, 3.4.1.7
With the --remote option that added submodule..branch (which
eventually landed with v8 of that series [4]), I initially imagined it
as the name of the local branch [5], but transitioned to imagining it
as the name of the remote-tracking branch in v5 of that series [6].
There were no major logical changes between v5 and v8. With the v8
version that landed in Git v1.8.2, submodule..br
[PATCH 010/14] git-resolve.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-resolve.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/examples/git-resolve.sh b/contrib/examples/git-resolve.sh
index 8f98142..48d0fc9 100755
--- a/contrib/examples/git-resolve.sh
+++ b/contrib/examples/git-resolve.sh
@@ -75,7 +75,7 @@ case "$common" in
GIT_INDEX_FILE=$G git read-tree -m $c $head $merge \
2>/dev/null || continue
# Count the paths that are unmerged.
- cnt=`GIT_INDEX_FILE=$G git ls-files --unmerged | wc -l`
+ cnt=$(GIT_INDEX_FILE=$G git ls-files --unmerged | wc -l)
if test $best_cnt -le 0 -o $cnt -le $best_cnt
then
best=$c
--
1.7.10.4
--
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 002/14] install-webdoc.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
Documentation/install-webdoc.sh |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/install-webdoc.sh b/Documentation/install-webdoc.sh
index 76d69a9..ed8b4ff 100755
--- a/Documentation/install-webdoc.sh
+++ b/Documentation/install-webdoc.sh
@@ -18,17 +18,17 @@ do
else
echo >&2 "# install $h $T/$h"
rm -f "$T/$h"
- mkdir -p `dirname "$T/$h"`
+ mkdir -p $(dirname "$T/$h")
cp "$h" "$T/$h"
fi
done
-strip_leading=`echo "$T/" | sed -e 's|.|.|g'`
+strip_leading=$(echo "$T/" | sed -e 's|.|.|g')
for th in \
"$T"/*.html "$T"/*.txt \
"$T"/howto/*.txt "$T"/howto/*.html \
"$T"/technical/*.txt "$T"/technical/*.html
do
- h=`expr "$th" : "$strip_leading"'\(.*\)'`
+ h=$(expr "$th" : "$strip_leading"'\(.*\)')
case "$h" in
RelNotes-*.txt | index.html) continue ;;
esac
--
1.7.10.4
--
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 006/14] git-fetch.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-fetch.sh |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/contrib/examples/git-fetch.sh b/contrib/examples/git-fetch.sh
index a314273..5540709 100755
--- a/contrib/examples/git-fetch.sh
+++ b/contrib/examples/git-fetch.sh
@@ -67,7 +67,7 @@ do
keep='-k -k'
;;
--depth=*)
- shallow_depth="--depth=`expr "z$1" : 'z-[^=]*=\(.*\)'`"
+ shallow_depth="--depth=$(expr "z$1" : 'z-[^=]*=\(.*\)')"
;;
--depth)
shift
@@ -262,12 +262,12 @@ fetch_per_ref () {
http://* | https://* | ftp://*)
test -n "$shallow_depth" &&
die "shallow clone with http not supported"
- proto=`expr "$remote" : '\([^:]*\):'`
+ proto=$(expr "$remote" : '\([^:]*\):')
if [ -n "$GIT_SSL_NO_VERIFY" ]; then
curl_extra_args="-k"
fi
if [ -n "$GIT_CURL_FTP_NO_EPSV" -o \
- "`git config --bool http.noEPSV`" = true ]; then
+ "$(git config --bool http.noEPSV)" = true ]; then
noepsv_opt="--disable-epsv"
fi
--
1.7.10.4
--
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 009/14] git-repack.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-repack.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/examples/git-repack.sh b/contrib/examples/git-repack.sh
index 7579331..f312405 100755
--- a/contrib/examples/git-repack.sh
+++ b/contrib/examples/git-repack.sh
@@ -49,7 +49,7 @@ do
shift
done
-case "`git config --bool repack.usedeltabaseoffset || echo true`" in
+case "$(git config --bool repack.usedeltabaseoffset || echo true)" in
true)
extra="$extra --delta-base-offset" ;;
esac
--
1.7.10.4
--
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 011/14] git-revert.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-revert.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/examples/git-revert.sh b/contrib/examples/git-revert.sh
index 6bf155c..7e2aad5 100755
--- a/contrib/examples/git-revert.sh
+++ b/contrib/examples/git-revert.sh
@@ -137,7 +137,7 @@ cherry-pick)
q
}'
- logmsg=`git show -s --pretty=raw --encoding="$encoding" "$commit"`
+ logmsg=$(git show -s --pretty=raw --encoding="$encoding" "$commit")
set_author_env=`echo "$logmsg" |
LANG=C LC_ALL=C sed -ne "$pick_author_script"`
eval "$set_author_env"
--
1.7.10.4
--
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 012/14] git-tag.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-tag.sh |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/examples/git-tag.sh b/contrib/examples/git-tag.sh
index 2c15bc9..1bd8f3c 100755
--- a/contrib/examples/git-tag.sh
+++ b/contrib/examples/git-tag.sh
@@ -156,7 +156,7 @@ prev=
if git show-ref --verify --quiet -- "refs/tags/$name"
then
test -n "$force" || die "tag '$name' already exists"
-prev=`git rev-parse "refs/tags/$name"`
+prev=$(git rev-parse "refs/tags/$name")
fi
shift
git check-ref-format "tags/$name" ||
--
1.7.10.4
--
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 004/14] git-clone.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-clone.sh | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/contrib/examples/git-clone.sh b/contrib/examples/git-clone.sh
index 547228e..b4c9376 100755
--- a/contrib/examples/git-clone.sh
+++ b/contrib/examples/git-clone.sh
@@ -40,7 +40,7 @@ eval "$(echo "$OPTIONS_SPEC" | git rev-parse --parseopt --
"$@" || echo exit $?)
get_repo_base() {
(
- cd "`/bin/pwd`" &&
+ cd "$(/bin/pwd)" &&
cd "$1" || cd "$1.git" &&
{
cd .git
@@ -50,7 +50,7 @@ get_repo_base() {
}
if [ -n "$GIT_SSL_NO_VERIFY" -o \
- "`git config --bool http.sslVerify`" = false ]; then
+ "$(git config --bool http.sslVerify)" = false ]; then
curl_extra_args="-k"
fi
@@ -70,7 +70,7 @@ clone_dumb_http () {
clone_tmp="$GIT_DIR/clone-tmp" &&
mkdir -p "$clone_tmp" || exit 1
if [ -n "$GIT_CURL_FTP_NO_EPSV" -o \
- "`git config --bool http.noEPSV`" = true ]; then
+ "$(git config --bool http.noEPSV)" = true ]; then
curl_extra_args="${curl_extra_args} --disable-epsv"
fi
http_fetch "$1/info/refs" "$clone_tmp/refs" ||
@@ -79,7 +79,7 @@ Perhaps git-update-server-info needs to be run there?"
test "z$quiet" = z && v=-v || v=
while read sha1 refname
do
- name=`expr "z$refname" : 'zrefs/\(.*\)'` &&
+ name=$(expr "z$refname" : 'zrefs/\(.*\)') &&
case "$name" in
*^*)continue;;
esac
@@ -88,7 +88,7 @@ Perhaps git-update-server-info needs to be run there?"
*) continue ;;
esac
if test -n "$use_separate_remote" &&
- branch_name=`expr "z$name" : 'zheads/\(.*\)'`
+ branch_name=$(expr "z$name" : 'zheads/\(.*\)')
then
tname="remotes/$origin/$branch_name"
else
@@ -100,7 +100,7 @@ Perhaps git-update-server-info needs to be run there?"
http_fetch "$1/HEAD" "$GIT_DIR/REMOTE_HEAD" ||
rm -f "$GIT_DIR/REMOTE_HEAD"
if test -f "$GIT_DIR/REMOTE_HEAD"; then
- head_sha1=`cat "$GIT_DIR/REMOTE_HEAD"`
+ head_sha1=$(cat "$GIT_DIR/REMOTE_HEAD")
case "$head_sha1" in
'ref: refs/'*)
;;
@@ -444,15 +444,15 @@ then
# a non-bare repository is always in separate-remote layout
remote_top="refs/remotes/$origin"
head_sha1=
- test ! -r "$GIT_DIR/REMOTE_HEAD" || head_sha1=`cat
"$GIT_DIR/REMOTE_HEAD"`
+ test ! -r "$GIT_DIR/REMOTE_HEAD" || head_sha1=$(cat
"$GIT_DIR/REMOTE_HEAD")
case "$head_sha1" in
'ref: refs/'*)
# Uh-oh, the remote told us (http transport done against
# new style repository with a symref HEAD).
# Ideally we should skip the guesswork but for now
# opt for minimum change.
- head_sha1=`expr "z$head_sha1" : 'zref: refs/heads/\(.*\)'`
- head_sha1=`cat "$GIT_DIR/$remote_top/$head_sha1"`
+ head_sha1=$(expr "z$head_sha1" : 'zref: refs/heads/\(.*\)')
+ head_sha1=$(cat "$GIT_DIR/$remote_top/$head_sha1")
;;
esac
@@ -467,7 +467,7 @@ then
while read name
do
test t = $done && continue
- branch_tip=`cat "$GIT_DIR/$remote_top/$name"`
+ branch_tip=$(cat "$GIT_DIR/$remote_top/$name")
if test "$head_sha1" = "$branch_tip"
then
echo "$name"
--
1.7.10.4
--
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 005/14] git-commit.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-commit.sh | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/contrib/examples/git-commit.sh b/contrib/examples/git-commit.sh
index 4aab1a6..5cafe2e 100755
--- a/contrib/examples/git-commit.sh
+++ b/contrib/examples/git-commit.sh
@@ -91,7 +91,7 @@ signoff=
force_author=
only_include_assumed=
untracked_files=
-templatefile="`git config commit.template`"
+templatefile="$(git config commit.template)"
while test $# != 0
do
case "$1" in
@@ -350,7 +350,7 @@ t,)
TMP_INDEX="$GIT_DIR/tmp-index$$"
W=
test -z "$initial_commit" && W=--with-tree=HEAD
- commit_only=`git ls-files --error-unmatch $W -- "$@"` || exit
+ commit_only=$(git ls-files --error-unmatch $W -- "$@") || exit
# Build a temporary index and update the real index
# the same way.
@@ -475,8 +475,8 @@ then
fi
if test '' != "$force_author"
then
- GIT_AUTHOR_NAME=`expr "z$force_author" : 'z\(.*[^ ]\) *<.*'` &&
- GIT_AUTHOR_EMAIL=`expr "z$force_author" : '.*\(<.*\)'` &&
+ GIT_AUTHOR_NAME=$(expr "z$force_author" : 'z\(.*[^ ]\) *<.*') &&
+ GIT_AUTHOR_EMAIL=$(expr "z$force_author" : '.*\(<.*\)') &&
test '' != "$GIT_AUTHOR_NAME" &&
test '' != "$GIT_AUTHOR_EMAIL" ||
die "malformed --author parameter"
@@ -489,7 +489,7 @@ then
rloga='commit'
if [ -f "$GIT_DIR/MERGE_HEAD" ]; then
rloga='commit (merge)'
- PARENTS="-p HEAD "`sed -e 's/^/-p /' "$GIT_DIR/MERGE_HEAD"`
+ PARENTS="-p HEAD "$(sed -e 's/^/-p /' "$GIT_DIR/MERGE_HEAD")
elif test -n "$amend"; then
rloga='commit (amend)'
PARENTS=$(git cat-file commit HEAD |
--
1.7.10.4
--
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 003/14] git-checkout.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-checkout.sh |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/contrib/examples/git-checkout.sh b/contrib/examples/git-checkout.sh
index d2c1f98..683cae7 100755
--- a/contrib/examples/git-checkout.sh
+++ b/contrib/examples/git-checkout.sh
@@ -222,7 +222,7 @@ else
# Match the index to the working tree, and do a three-way.
git diff-files --name-only | git update-index --remove --stdin &&
- work=`git write-tree` &&
+ work=$(git write-tree) &&
git read-tree $v --reset -u $new || exit
eval GITHEAD_$new='${new_name:-${branch:-$new}}' &&
@@ -233,7 +233,7 @@ else
# Do not register the cleanly merged paths in the index yet.
# this is not a real merge before committing, but just carrying
# the working tree changes along.
- unmerged=`git ls-files -u`
+ unmerged=$(git ls-files -u)
git read-tree $v --reset $new
case "$unmerged" in
'') ;;
@@ -269,7 +269,7 @@ if [ "$?" -eq 0 ]; then
fi
if test -n "$branch"
then
- old_branch_name=`expr "z$oldbranch" : 'zrefs/heads/\(.*\)'`
+ old_branch_name=$(expr "z$oldbranch" : 'zrefs/heads/\(.*\)')
GIT_DIR="$GIT_DIR" git symbolic-ref -m "checkout: moving from
${old_branch_name:-$old} to $branch" HEAD "refs/heads/$branch"
if test -n "$quiet"
then
@@ -282,7 +282,7 @@ if [ "$?" -eq 0 ]; then
fi
elif test -n "$detached"
then
- old_branch_name=`expr "z$oldbranch" : 'zrefs/heads/\(.*\)'`
+ old_branch_name=$(expr "z$oldbranch" : 'zrefs/heads/\(.*\)')
git update-ref --no-deref -m "checkout: moving from
${old_branch_name:-$old} to $arg" HEAD "$detached" ||
die "Cannot detach HEAD"
if test -n "$detach_warn"
--
1.7.10.4
--
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] send-email: recognize absolute path on Windows
Junio C Hamano writes:
> Erik Faye-Lund writes:
>
>> So let's manually check for these in that case, and fall back to
>> the File::Spec-helper on other platforms (e.g Win32 with native
>> Perl)
>> ...
>> +sub file_name_is_absolute {
>> +my ($path) = @_;
>> +
>> +# msys does not grok DOS drive-prefixes
>> +if ($^O eq 'msys') {
>> +return ($path =~ m#^/# || $path =~ m#[a-zA-Z]\:#)
>
> Shouldn't the latter also be anchored at the beginning of the string
> with a leading "^"?
>
>> +}
>> +
>> +require File::Spec::Functions;
>> +return File::Spec::Functions::file_name_is_absolute($path);
>
> We already "use File::Spec qw(something else)" at the beginning, no?
> Why not throw file_name_is_absolute into that qw() instead?
Ahh, OK, if you did so, you won't have any place to hook the "only
on msys do this" trick into.
It somehow feels somewhat confusing that we define a sub with the
same name as the system one, while not overriding it entirely but
delegate back to the system one. I am debating myself if it is more
obvious if it is done this way:
use File::Spec::Functions qw(file_name_is_absolute);
if ($^O eq 'msys') {
sub file_name_is_absolute {
return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
}
}
>> # Returns 1 if the message was sent, and 0 otherwise.
>> # In actuality, the whole program dies when there
>> # is an error sending a message.
>> @@ -1197,7 +1209,7 @@ X-Mailer: git-send-email $gitversion
>>
>> if ($dry_run) {
>> # We don't want to send the email.
>> -} elsif ($smtp_server =~ m#^/#) {
>> +} elsif (file_name_is_absolute($smtp_server)) {
>> my $pid = open my $sm, '|-';
>> defined $pid or die $!;
>> if (!$pid) {
>> @@ -1271,7 +1283,7 @@ X-Mailer: git-send-email $gitversion
>> printf (($dry_run ? "Dry-" : "")."Sent %s\n", $subject);
>> } else {
>> print (($dry_run ? "Dry-" : "")."OK. Log says:\n");
>> -if ($smtp_server !~ m#^/#) {
>> +if (!file_name_is_absolute($smtp_server)) {
>> print "Server: $smtp_server\n";
>> print "MAIL FROM:<$raw_from>\n";
>> foreach my $entry (@recipients) {
>> --
>> 1.9.0.msysgit.0
>>
>> --
>
> --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 008/14] git-merge.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-merge.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/examples/git-merge.sh b/contrib/examples/git-merge.sh
index a5e42a9..7e40f40 100755
--- a/contrib/examples/git-merge.sh
+++ b/contrib/examples/git-merge.sh
@@ -341,7 +341,7 @@ case "$use_strategies" in
'')
case "$#" in
1)
- var="`git config --get pull.twohead`"
+ var="$(git config --get pull.twohead)"
if test -n "$var"
then
use_strategies="$var"
@@ -349,7 +349,7 @@ case "$use_strategies" in
use_strategies="$default_twohead_strategies"
fi ;;
*)
- var="`git config --get pull.octopus`"
+ var="$(git config --get pull.octopus)"
if test -n "$var"
then
use_strategies="$var"
--
1.7.10.4
--
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 013/14] t9360-mw-to-git-clone.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/mw-to-git/t/t9360-mw-to-git-clone.sh | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
index 811a90c..22f069d 100755
--- a/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
+++ b/contrib/mw-to-git/t/t9360-mw-to-git-clone.sh
@@ -191,10 +191,10 @@ test_expect_success 'Git clone works with the shallow
option' '
test_path_is_file mw_dir_11/Main_Page.mw &&
(
cd mw_dir_11 &&
- test `git log --oneline Nyan.mw | wc -l` -eq 1 &&
- test `git log --oneline Foo.mw | wc -l` -eq 1 &&
- test `git log --oneline Bar.mw | wc -l` -eq 1 &&
- test `git log --oneline Main_Page.mw | wc -l ` -eq 1
+ test $(git log --oneline Nyan.mw | wc -l) -eq 1 &&
+ test $(git log --oneline Foo.mw | wc -l) -eq 1 &&
+ test $(git log --oneline Bar.mw | wc -l) -eq 1 &&
+ test $(git log --oneline Main_Page.mw | wc -l ) -eq 1
) &&
wiki_check_content mw_dir_11/Nyan.mw Nyan &&
wiki_check_content mw_dir_11/Foo.mw Foo &&
@@ -218,9 +218,9 @@ test_expect_success 'Git clone works with the shallow
option with a delete page'
test_path_is_file mw_dir_12/Main_Page.mw &&
(
cd mw_dir_12 &&
- test `git log --oneline Nyan.mw | wc -l` -eq 1 &&
- test `git log --oneline Bar.mw | wc -l` -eq 1 &&
- test `git log --oneline Main_Page.mw | wc -l ` -eq 1
+ test $(git log --oneline Nyan.mw | wc -l) -eq 1 &&
+ test $(git log --oneline Bar.mw | wc -l) -eq 1 &&
+ test $(git log --oneline Main_Page.mw | wc -l ) -eq 1
) &&
wiki_check_content mw_dir_12/Nyan.mw Nyan &&
wiki_check_content mw_dir_12/Bar.mw Bar &&
--
1.7.10.4
--
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 014/14] t9362-mw-to-git-utf8.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
b/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
index 37021e2..6b0dbda 100755
--- a/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
+++ b/contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh
@@ -70,8 +70,8 @@ test_expect_success 'The shallow option works with accents' '
test_path_is_file mw_dir_4/Main_Page.mw &&
(
cd mw_dir_4 &&
- test `git log --oneline Néoà.mw | wc -l` -eq 1 &&
- test `git log --oneline Main_Page.mw | wc -l ` -eq 1
+ test $(git log --oneline Néoà.mw | wc -l) -eq 1 &&
+ test $(git log --oneline Main_Page.mw | wc -l ) -eq 1
) &&
wiki_check_content mw_dir_4/Néoà.mw Néoà &&
wiki_check_content mw_dir_4/Main_Page.mw Main_Page
--
1.7.10.4
--
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 007/14] git-ls-remote.sh: use the $( ... ) construct for command substitution
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto
---
contrib/examples/git-ls-remote.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/examples/git-ls-remote.sh
b/contrib/examples/git-ls-remote.sh
index fec70bb..2aa89a7 100755
--- a/contrib/examples/git-ls-remote.sh
+++ b/contrib/examples/git-ls-remote.sh
@@ -55,11 +55,11 @@ tmpdir=$tmp-d
case "$peek_repo" in
http://* | https://* | ftp://* )
if [ -n "$GIT_SSL_NO_VERIFY" -o \
- "`git config --bool http.sslVerify`" = false ]; then
+ "$(git config --bool http.sslVerify)" = false ]; then
curl_extra_args="-k"
fi
if [ -n "$GIT_CURL_FTP_NO_EPSV" -o \
- "`git config --bool http.noEPSV`" = true ]; then
+ "$(git config --bool http.noEPSV)" = true ]; then
curl_extra_args="${curl_extra_args} --disable-epsv"
fi
curl -nsf $curl_extra_args --header "Pragma: no-cache"
"$peek_repo/info/refs" ||
--
1.7.10.4
--
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: On "interpret-trailers" standalone tool
Christian Couder writes: > If the input comes from stdin, then I agree that the command should be > a filter, so its output should be on stdout. But if the input comes > from files given as arguments, then I would say that the command > should behave as an editor and by default it should edit its input > file inplace. I do not see where that "should" comes from. I am looking at this more from the angle of obtaining a useful building block, while you seem to be thinking of this as a specialized tool for a narrow set of specifkc tasks. Thinking of examples in "sort" and "sed", I would say "read multiple files, futz with the contents and spit out a single output stream" is the way people expect a command to operate without being told to operate in some other way. Overwriting existing files should never be the default for safety---otherwise you would require people who want safety to do something like: cp realfile tmp futz tmp verify tmp mv tmp realfile On the other hand, a usual "sort/sed/cat"-like command, even without the "in-place edit" option, can be used as in-place with mv realfile tmp futz tmp >realfile easily, and is more flexible as a building block. Of course, that does not rule out an option to work in-place (e.g. in a similar way to "sort -o file file", or "perl -i -e 'y/a-z/A-Z/' frotz nitfol"). > Its input and output files should be different only if > it is given one or more special option, > > Otherwise the example you gave: > > $ git format-patch -5 --cover-letter -o +my-series/ my-topic > $ git interpret-trailers "some args" ./+my-series/0*.patch > > would result in having on stdout all the patches edited by "git > interpret-trailers". Didn't I mention that I do not mind "-i" already if in-place edit is desired? Add "-i" to the command line arguments among "some args", and your complaints will disappear, no? -- 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] config.c: mark die_bad_number as NORETURN
Jeff King writes:
> On Wed, Apr 16, 2014 at 06:38:19PM +0200, Torsten Bögershausen wrote:
>
>> > Does that also silence the warning?
>> >
>> This works under gcc 4.2.1 Mac OS: the warning is away.
>
> Thanks. I couldn't test myself, as I could not get gcc to generate the
> warning in the first place, but I do not have anything as old as 4.2 on
> hand.
>
> Here it is with a commit message.
>
> -- >8 --
> Subject: config.c: mark die_bad_number as NORETURN
>
> This can help avoid -Wuninitialized false positives in
> git_config_int and git_config_ulong, as the compiler now
> knows that we do not return "ret" if we hit the error
> codepath.
Thanks for clearly solving the issue and describing the solution.
Will queue.
> Signed-off-by: Jeff King
> ---
> config.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/config.c b/config.c
> index 6821cef..a30cb5c 100644
> --- a/config.c
> +++ b/config.c
> @@ -557,6 +557,7 @@ int git_parse_ulong(const char *value, unsigned long *ret)
> return 1;
> }
>
> +NORETURN
> static void die_bad_number(const char *name, const char *value)
> {
> const char *reason = errno == ERANGE ?
--
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: gitignore vs. exclude vs assume-unchanged?
a...@bellandwhistle.net writes: > Any clarification on the differences much appreciated: > > http://stackoverflow.com/questions/23097368/git-ignore-vs-exclude-vs-assume-unchanged/23097509 Please don't force people to refer to external site. The .gitignore and .git/info/exclude are the two UIs to invoke the same mechanism. In-tree .gitignore are to be shared among project members (i.e. everybody working on the project should consider the paths that match the ignore pattern in there as cruft). On the other hand, .git/info/exclude is meant for personal ignore patterns (i.e. you, while working on the project, consider them as cruft). Assume-unchanged should not be abused for an ignore mechanism. It is "I know my filesystem operations are slow. I'll promise Git that I won't change these paths by making them with that bit---that way, Git does not have to check if I changed things in there every time I ask for 'git status' output". It does not mean anything other than that. Especially, it is *not* a promise by Git that Git will always consider these paths are unmodified---if Git can determine a path that is marked as assume-unchanged has changed without incurring extra lstat(2) cost, it reserves the right to report that the path *has been* modified (as a result, "git commit -a" is free to commit that change). -- 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
gitignore vs. exclude vs assume-unchanged?
Any clarification on the differences much appreciated: http://stackoverflow.com/questions/23097368/git-ignore-vs-exclude-vs-assume-unchanged/23097509 -- 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 1/3] rebase: avoid non-function use of "return" on FreeBSD
Junio C Hamano writes:
> "Kyle J. McKay" writes:
>
>> If I'm the only one getting a wrong meaning from the comments, then no
>> reason to change them.
>
> I agree that the description does not read well with the work-around
> already there. I am not sure what would be a better way to phrase
> it, though.
Here is a tentative rewrite at the beginning and the end of rebase-am:
# The whole contents of the file is run by dot-sourcing this
# file from inside a shell function. It used to be that
# "return"s we see below were not inside any function, and
# expected to return from the function that dot-sourced us.
#
# However, FreeBSD /bin/sh misbehaves on such a construct and
# continues to run the statements that follow such a "return".
# As a work-around, we introduce an extra layer of a function
# here, and immediately call it after defining it.
git_rebase__am () {
...
}
# ... and then we call the whole thing.
git_rebase__am
By the way, you have this in your log message:
... the git-rebase--*.sh scripts have used a "return" to return
from the "dot" command that runs them. While this is allowed by
POSIX,...
Is it "this is allowed", or is it "this should be the way and shells
that do not do so are buggy"?
--
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 1/3] rebase: avoid non-function use of "return" on FreeBSD
Junio C Hamano writes: > By the way, you have this in your log message: > > ... the git-rebase--*.sh scripts have used a "return" to return > from the "dot" command that runs them. While this is allowed by > POSIX,... > > > Is it "this is allowed", or is it "this should be the way and shells > that do not do so are buggy"? Answering myself... The only "unspecified" I see is this: If the shell is not currently executing a function or dot script, the results are unspecified. which clearly does not apply to the version before this patch (we are executing a dot script). And The return utility shall cause the shell to stop executing the current function or dot script. would mean that we are correct to expect that "should not get here" is not reached, as the "return 5" would cause the shell to stop executing the dot script there. So "while this is allowed by POSIX" may be a bit misleading and needs to be reworded, I guess? -- 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: On "interpret-trailers" standalone tool
Junio C Hamano writes: > ... I am looking at this > more from the angle of obtaining a useful building block, while you > seem to be thinking of this as a specialized tool for a narrow set > of specifkc tasks. By the way, I am speaking with a bitter experience of designing the original "format-patch" command line parameters, thinking that "This is a specialized tool to let me send what Linus hasn't picked up yet, so the command should take where Linus is and where I am". Not using the A..B syntax turned out to be a poor choice but that realization came long after the ship sailed---back then, A..B syntax was relatively new and it was unclear that it would become the universal syntax we use throughout the system to denote a range of commits in the DAG. The design mistake for starting at a specialized tool for a narrow set of specific tasks took considerable effort to retrofit the general syntax while not breaking the traditional usage. I am being cautious here because I do not see us making the same mistake. -- 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: On "interpret-trailers" standalone tool
Junio C Hamano writes: > ... I am being > cautious here because I do not see us making the same mistake. s/do not/& want to/ Sorry for the noise. -- 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 2/2] commit.c: check for lock error and return early
Move the check for the lock failure to happen immediately after
lock_any_ref_for_update().
Previously the lock and the check-if-lock-failed was separated by a handful
of string manipulation statements.
Moving the check to occur immediately after the failed lock makes the
code slightly easier to read and makes it follow the pattern of
try-to-take-a-lock()
if (check-if-lock-failed){
error
}
---
builtin/commit.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index d9550c5..c6320f1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1672,6 +1672,10 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
? NULL
: current_head->object.sha1,
0, NULL);
+ if (!ref_lock) {
+ rollback_index_files();
+ die(_("cannot lock HEAD ref"));
+ }
nl = strchr(sb.buf, '\n');
if (nl)
@@ -1681,10 +1685,6 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
- if (!ref_lock) {
- rollback_index_files();
- die(_("cannot lock HEAD ref"));
- }
if (write_ref_sha1(ref_lock, sha1, sb.buf) < 0) {
rollback_index_files();
die(_("cannot update HEAD ref"));
--
1.9.1.504.g5a62d94
--
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/2] Check for lock failures early
Callers outside of refs.c use either lock_ref_sha1() or lock_any_ref_for_update() to lock a ref during an update. Two of these places we do not immediately check the lock for failure making reading the code harder. One place we do some unrelated string manipulation fucntions before we check for failure and the other place we rely on that write_ref_sha1() will check the lock for failure and return an error. These two patches updates these two places so that we immediately check the lock for failure and act on it. It does not change any functionality or logic but makes the code easier to read by being more consistent. Version 2: * Simplify the return on error case in sequencer.c. Ronnie Sahlberg (2): sequencer.c: check for lock failure and bail early in fast_forward_to commit.c: check for lock error and return early builtin/commit.c | 8 sequencer.c | 4 2 files changed, 8 insertions(+), 4 deletions(-) -- 1.9.1.504.g5a62d94 -- 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/2] sequencer.c: check for lock failure and bail early in fast_forward_to
Change fast_forward_to() to check if locking the ref failed, print a nice
error message and bail out early.
The old code did not check if ref_lock was NULL and relied on the fact
that the write_ref_sha1() would safely detect this condition and set the
return variable ret to indicate an error.
While that is safe, it makes the code harder to read for two reasons:
* Inconsistency. Almost all other places we do check the lock for NULL
explicitely, so the naive reader is confused "why don't we check here".
* And relying on write_ref_sha1() to detect and return an error for when
a previous lock_any_ref_for_update() feels obfuscated.
This change should not change any functionality or logic
aside from adding an extra error message when this condition is triggered.
(write_ref_sha1() returns an error silently for this condition)
Signed-off-by: Ronnie Sahlberg
---
sequencer.c | 4
1 file changed, 4 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index bde5f04..0a80c58 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -281,8 +281,12 @@ static int fast_forward_to(const unsigned char *to, const
unsigned char *from,
exit(1); /* the callee should have complained already */
ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from,
0, NULL);
+ if (!ref_lock)
+ return error(_("Failed to lock HEAD during fast_forward_to"));
+
strbuf_addf(&sb, "%s: fast-forward", action_name(opts));
ret = write_ref_sha1(ref_lock, to, sb.buf);
+
strbuf_release(&sb);
return ret;
}
--
1.9.1.504.g5a62d94
--
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] git-rebase: Print name of rev when using shorthand
Junio C Hamano writes:
>> Furthermore, were we to translate "@{-1}", does that mean we
>> should also translate "@{-2}" or prior?
>
> Surely, why not. If a user is so forgetful to need help remembering
> where s/he was immediately before, wouldn't it be more helpful to
> give "here is where you were" reminder for older ones to allow them
> to double check they specified the right thing and spot possible
> mistakes?
After re-reading the proposed log message of your v2, I notice one
thing:
The output from a successful invocation of the shorthand command
"git rebase -" is something like "Fast-forwarded HEAD to @{-1}",
which includes a relative reference to a revision. Other
commands that use the shorthand "-", such as "git checkout -",
typically display the symbolic name of the revision.
While the above is not incorrect per-se, have you considered _why_
it is a good thing to show the symbolic name in the first place?
Giving the symbolic name 'master' is good because it is possible
that the user thought the previous branch was 'frotz', forgetting
that another branch was checked out tentatively in between, and the
user ended up rebasing on top of a wrong branch. Telling what that
previous branch is is a way to help user spot such a potential
mistake. So I am all for making "rebase -" report what concrete
branch the branch was replayed on top of, and consider it an incomplete
improvement if "rebase @{-1}" (or "rebase @{-2}") did not get the
same help---especially when I know that the underlying mechanism you
would use to translate @{-1} back to the concrete branch name is the
same for both cases anyway.
By the way, here is a happy tangent. I was pleasantly surprised to
see what this procedure produced:
$ git checkout -b ef/send-email-absolute-path maint
$ git am -s3c a-patch-by-erik-on-different-topic
$ git checkout bg/rebase-off-of-previous-branch
$ git am -s3c your-v2-patch
$ git checkout jch
$ git merge --no-edit -
$ git merge --no-edit @{-2}
$ git log --first-parent -2 | grep "Merge branch"
Both short-hands are turned into concrete branch names, as they
should ;-)
--
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] Update SVN.pm
Stepan Kasal writes:
> From: RomanBelinsky
> Date: Tue, 11 Feb 2014 18:23:02 +0200
>
> fix parsing error for dates like:
> 2014-01-07T5:58:36.048176Z
> previous regex can parse only:
> 2014-01-07T05:58:36.048176Z
> reproduced in my svn repository during conversion.
Interesting. What other strange forms can they record in their
repositories, I have to wonder. Can they do
2014-01-07T5:8:6.048176Z
for example? I am wondering if it is simpler and less error prone
to turn all these "we only accept two digits" into "\d+" not only
for the hour part but also minute and second parts.
> Signed-off-by: Stepan Kasal
> ---
> perl/Git/SVN.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
> index a59564f..09cff13 100644
> --- a/perl/Git/SVN.pm
> +++ b/perl/Git/SVN.pm
> @@ -1321,7 +1321,7 @@ sub get_untracked {
> sub parse_svn_date {
> my $date = shift || return '+ 1970-01-01 00:00:00';
> my ($Y,$m,$d,$H,$M,$S) = ($date =~ /^(\d{4})\-(\d\d)\-(\d\d)T
> - (\d\d)\:(\d\d)\:(\d\d)\.\d*Z$/x) or
> + (\d\d?)\:(\d\d)\:(\d\d)\.\d*Z$/x) or
>croak "Unable to parse date: $date\n";
> my $parsed_date;# Set next.
--
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 0/3] Make update refs more atomic
Ronnie Sahlberg writes:
> Currently any locking of refs in a transaction only happens during the commit
> phase. I think it would be useful to have a mechanism where you could
> optionally take out locks for the involved refs early during the transaction.
> So that simple callers could continue using
> ref_transaction_begin()
> ref_transaction_create|update|delete()*
> ref_transaction_commit()
>
> but, if a caller such as walker_fetch() could opt to do
> ref_transaction_begin()
> ref_transaction_lock_ref()*
> ...do stuff...
> ref_transaction_create|update|delete()*
> ref_transaction_commit()
>
> In this second case ref_transaction_commit() would only take out any locks
> that
> are missing during the 'lock the refs" loop.
>
> Suggestion 1: Add a ref_transaction_lock_ref() to allow locking a ref
> early during
> a transaction.
Hmph.
I am not sure if that is the right way to go, or instead change all
create/update/delete to take locks without adding a new primitive.
> A second idea is to change the signatures for
> ref_transaction_create|update|delete()
> slightly and allow them to return errors early.
> We can check for things like add_update() failing, check that the
> ref-name looks sane,
> check some of the flags, like if has_old==true then old sha1 should
> not be NULL or 0{40}, etc.
>
> Additionally for robustness, if any of these functions detect an error
> we can flag this in the
> transaction structure and take action during ref_transaction_commit().
> I.e. if a ref_transaction_update had a hard failure, do not allow
> ref_transaction_commit()
> to succeed.
>
> Suggestion 2: Change ref_transaction_create|delete|update() to return an int.
> All callers that use these functions should check the function for error.
I think that is a very sensible thing to do.
The details of determining "this cannot possibly succeed" may change
(for example, if we have them take locks at the point of
create/delete/update, a failure to lock may count as an early
error).
Is there any reason why this should be conditional (i.e. you said
"allow them to", implying that the early failure is optional)?
> Suggestion 3: remove the qsort and check for duplicates in
> ref_transaction_commit()
> Since we are already taking out a lock for each ref we are updating
> during the transaction
> any duplicate refs will fail the second attempt to lock the same ref which
> will
> implicitly make sure that a transaction will not change the same ref twice.
I do not know if I care about the implementation detail of "do we
have a unique set of update requests?". While I do not see a strong
need for one transaction to touch the same ref twice (e.g. create to
point at commit A and update it to point at commit B), I do not see
why we should forbid such a use in the future.
Just some of my knee-jerk reactions.
--
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 00/25] Lockfile correctness and refactoring
On 04/15/2014 08:40 PM, Torsten Bögershausen wrote:
> refs.c:
> int close_ref(struct ref_lock *lock)
> {
> if (close_lock_file(lock->lk))
> return -1;
> lock->lock_fd = -1;
> return 0;
> }
>
> When the close() fails, fd is still >= 0, even if the file is closed.
>
> Could it be written like this ?
>
> int close_ref(struct ref_lock *lock)
> {
> return close_lock_file(lock->lk);
> }
It seem to me it would be better to set lock->lock_fd to -1 regardless
of whether close_lock_file() succeeds. Or maybe this field is not even
needed, and instead lock->lk->fd should be used.
This is a bit beyond the scope of this patch series, but I will put it
on my todo list.
> =
> lockfile.c, line 49
> * - filename holds the filename of the lockfile
>
> I think we have a strbuf here? (which is a good thing),
> should we write:
> * - strbuf filename holds the filename of the lockfile
> --
> (and at some places filename[0] is mentioned,
> should that be filename.buf[0] ?)
I think it is OK to speak of a strbuf as "holding" a filename (what else
would that construct mean?
But you are correct that the comments shouldn't speak of filename[0]
anymore. I will fix it.
> =
> int commit_lock_file(struct lock_file *lk)
> {
> static struct strbuf result_file = STRBUF_INIT;
> int err;
>
> if (lk->fd >= 0 && close_lock_file(lk))
> return -1;
> ##What happens if close() fails and close_lock_file() returns != 0?
> ##Is the lk now in a good shaped state?
> I think the file which had been open by lk->fd is in an unkown state,
> and should not be used for anything.
> When I remember it right, an error on close() can mean "the file could not
> be written and closed as expected, it can be truncated or corrupted.
> This can happen on a network file system like NFS, or probably even other FS.
> For me the failing of close() means I smell a need for a rollback.
Yes, this is a good catch. I think a rollback should definitely be done
in this case. I will fix it.
In fact, I'm wondering whether it would be appropriate for
close_lock_file() itself to do a rollback if close() fails. I guess I
will first have to audit callers to make sure that they don't try to use
lock_file::filename after a failed close_lock_file() (e.g., for
generating an error message).
> Please treat my comments more than questions rather than answers,
> thanks for an interesting reading
Thanks for your helpful comments!
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] Unicode: update of combining code points
On 2014-04-16 12.51, Kevin Bracey wrote: > On 16/04/2014 07:48, Torsten Bögershausen wrote: >> On 15.04.14 21:10, Peter Krefting wrote: >>> Torsten Bögershausen: >>> diff --git a/utf8.c b/utf8.c index a831d50..77c28d4 100644 --- a/utf8.c +++ b/utf8.c >>> Is there a script that generates this code from the Unicode database files, >>> or did you hand-update it? >>> >> Some of the code points which have "0 length on the display" are called >> "combining", others are called "vowels" or "accents". >> E.g. 5BF is not marked any of them, but if you look at the glyph, it should >> be combining (please correct me if that is wrong). > > Indeed it is combining (more specifically it has General Category > "Nonspacing_Mark" = "Mn"). > >> >> If I could have found a file which indicates for each code point, what it >> is, I could write a script. >> > > The most complete and machine-readable data are in these files: > > http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt > http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt > > The general categories can also be seen more legibly in: > > http://www.unicode.org/Public/UCD/latest/ucd/extracted/DerivedGeneralCategory.txt > > For docs, see: > > http://www.unicode.org/reports/tr44/ > http://www.unicode.org/reports/tr11/ > http://www.unicode.org/ucd/ > > The existing utf8.c comments describe the attributes being selected from the > tables (general categories "Cf","Mn","Me", East Asian Width "W", "F"). And > they suggest that the combining character table was originally auto-generated > from UnicodeData.txt with a "uniset" tool. Presumably this? > > https://github.com/depp/uniset > > The fullwidth-checking code looks like it was done by hand, although > apparently uniset can process EastAsianWidth.txt. > > Kevin Excellent, thanks for the pointers. Running the script below shows that "0X00AD SOFT HYPHEN" should have zero length (and some others too). I wonder if that is really the case, and which one of the last 2 lines in the script is the right one. What does this mean for us: "Cf Format a format control character" #!/bin/sh if ! test -f UnicodeData.txt; then wget http://www.unicode.org/Public/UCD/latest/ucd/UnicodeData.txt fi && if ! test -f EastAsianWidth.txt; then wget http://www.unicode.org/Public/UCD/latest/ucd/EastAsianWidth.txt fi if ! test -f DerivedGeneralCategory.txt; then wget http://www.unicode.org/Public/UCD/latest/ucd/extracted/DerivedGeneralCategory.txt fi && if ! test -d uniset; then git clone https://github.com/tboegi/uniset.git fi && ( cd uniset && if ! test -x uniset; then autoreconf -i && ./configure --enable-warnings=-Werror CFLAGS='-O0 -ggdb' fi && make ) && UNICODE_DIR=. ./uniset/uniset --32 cat:Me,Mn,Cf #UNICODE_DIR=. ./uniset/uniset --32 cat:Me,Mn > > -- > 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 -- 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] tag: add -i and --introduced modifier for --contains
From: "Luis R. Rodriguez"
Upstream Linux kernel commit c5905afb was introduced on v3.4 but
git describe --contains yields v3.5 while if we use git to look
for the first parent with git describe --first-parent yields
v3.3. The reason for this seems to be that the merge commit that
introduced c5905afb was based on v3.3. At least for --contains
its unclear to me why we get v3.5, the result is not intuitive,
as for --first-parent the issue is that the first parent actually
*is* v3.3. The easiest way to address this it to rely on on the
git tag --contains implmenetation and add a modifier that specifies
you want the tag that first introduced the specified commit.
mcgrof@ergon ~/linux (git::master)$ git tag -i --contains c5905afb
v3.4
mcgrof@ergon ~/linux (git::master)$ git tag --introduced --contains c5905afb
v3.4
Cc: Jiri Slaby
Cc: Andreas Schwab
Cc: Jan Kara
Signed-off-by: Luis R. Rodriguez
---
builtin/tag.c | 21 -
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index 6c7c6bd..65a939b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -21,7 +21,7 @@
static const char * const git_tag_usage[] = {
N_("git tag [-a|-s|-u ] [-f] [-m |-F ]
[]"),
N_("git tag -d ..."),
- N_("git tag -l [-n[]] [--contains ] [--points-at ]
"
+ N_("git tag -l [-n[]] [--contains ] [ -i | --introduced
--contains ] [--points-at ] "
"\n\t\t[...]"),
N_("git tag -v ..."),
NULL
@@ -195,13 +195,18 @@ static int sort_by_version(const void *a_, const void *b_)
}
static int list_tags(const char **patterns, int lines,
-struct commit_list *with_commit, int sort)
+struct commit_list *with_commit, int sort,
+int introduced)
{
struct tag_filter filter;
filter.patterns = patterns;
filter.lines = lines;
- filter.sort = sort;
+ if (introduced) {
+ sort = VERCMP_SORT;
+ filter.sort = sort;
+ } else
+ filter.sort = sort;
filter.with_commit = with_commit;
memset(&filter.tags, 0, sizeof(filter.tags));
filter.tags.strdup_strings = 1;
@@ -216,8 +221,11 @@ static int list_tags(const char **patterns, int lines,
for (i = filter.tags.nr - 1; i >= 0; i--)
printf("%s\n", filter.tags.items[i].string);
else
- for (i = 0; i < filter.tags.nr; i++)
+ for (i = 0; i < filter.tags.nr; i++) {
printf("%s\n", filter.tags.items[i].string);
+ if (introduced)
+ break;
+ }
string_list_clear(&filter.tags, 0);
}
return 0;
@@ -493,6 +501,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
int cmdmode = 0, sort = 0;
+ int introduced = 0;
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
@@ -511,6 +520,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
N_("tag message"), parse_msg_arg),
OPT_FILENAME('F', "file", &msgfile, N_("read message from
file")),
OPT_BOOL('s', "sign", &opt.sign, N_("annotated and GPG-signed
tag")),
+ OPT_BOOL('i', "introduced", &introduced, N_("print the first
tag that introduced the commit")),
OPT_STRING(0, "cleanup", &cleanup_arg, N_("mode"),
N_("how to strip spaces and #comments from message")),
OPT_STRING('u', "local-user", &keyid, N_("key-id"),
@@ -576,7 +586,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
}
if (lines != -1 && sort)
die(_("--sort and -n are incompatible"));
- ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit,
sort);
+ ret = list_tags(argv, lines == -1 ? 0 : lines, with_commit,
+ sort, introduced);
if (column_active(colopts))
stop_column_filter();
return ret;
--
1.9.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/3] Make update refs more atomic
On Wed, Apr 16, 2014 at 12:31 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> Currently any locking of refs in a transaction only happens during the commit
>> phase. I think it would be useful to have a mechanism where you could
>> optionally take out locks for the involved refs early during the transaction.
>> So that simple callers could continue using
>> ref_transaction_begin()
>> ref_transaction_create|update|delete()*
>> ref_transaction_commit()
>>
>> but, if a caller such as walker_fetch() could opt to do
>> ref_transaction_begin()
>> ref_transaction_lock_ref()*
>> ...do stuff...
>> ref_transaction_create|update|delete()*
>> ref_transaction_commit()
>>
>> In this second case ref_transaction_commit() would only take out any locks
>> that
>> are missing during the 'lock the refs" loop.
>>
>> Suggestion 1: Add a ref_transaction_lock_ref() to allow locking a ref
>> early during
>> a transaction.
>
> Hmph.
>
> I am not sure if that is the right way to go, or instead change all
> create/update/delete to take locks without adding a new primitive.
ack.
>
>> A second idea is to change the signatures for
>> ref_transaction_create|update|delete()
>> slightly and allow them to return errors early.
>> We can check for things like add_update() failing, check that the
>> ref-name looks sane,
>> check some of the flags, like if has_old==true then old sha1 should
>> not be NULL or 0{40}, etc.
>>
>> Additionally for robustness, if any of these functions detect an error
>> we can flag this in the
>> transaction structure and take action during ref_transaction_commit().
>> I.e. if a ref_transaction_update had a hard failure, do not allow
>> ref_transaction_commit()
>> to succeed.
>>
>> Suggestion 2: Change ref_transaction_create|delete|update() to return an int.
>> All callers that use these functions should check the function for error.
>
> I think that is a very sensible thing to do.
>
> The details of determining "this cannot possibly succeed" may change
> (for example, if we have them take locks at the point of
> create/delete/update, a failure to lock may count as an early
> error).
>
> Is there any reason why this should be conditional (i.e. you said
> "allow them to", implying that the early failure is optional)?
It was poor wording on my side. Checking for the ref_transaction_*()
return for error should be mandatory (modulo bugs).
But a caller could be buggy and fail to check properly.
It would be very cheap to detect this condition in
ref_transaction_commit() which could then do
die("transaction commit called for errored transaction");
which would make it easy to spot this kind of bugs.
>
>> Suggestion 3: remove the qsort and check for duplicates in
>> ref_transaction_commit()
>> Since we are already taking out a lock for each ref we are updating
>> during the transaction
>> any duplicate refs will fail the second attempt to lock the same ref which
>> will
>> implicitly make sure that a transaction will not change the same ref twice.
>
> I do not know if I care about the implementation detail of "do we
> have a unique set of update requests?". While I do not see a strong
> need for one transaction to touch the same ref twice (e.g. create to
> point at commit A and update it to point at commit B), I do not see
> why we should forbid such a use in the future.
>
ack.
--
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 0/3] Make update refs more atomic
Ronnie Sahlberg writes: >> I am not sure if that is the right way to go, or instead change all >> create/update/delete to take locks without adding a new primitive. > > ack. Hmph. When I say "I am not sure", "I dunno", etc., I do mean it. Did you mean by "Ack" "I do not know, either", or "I think it is better to take locks early everywhere"? -- 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 0/3] Make update refs more atomic
On 04/16/2014 09:31 PM, Junio C Hamano wrote:
> Ronnie Sahlberg writes:
>
>> Currently any locking of refs in a transaction only happens during the commit
>> phase. I think it would be useful to have a mechanism where you could
>> optionally take out locks for the involved refs early during the transaction.
>> So that simple callers could continue using
>> ref_transaction_begin()
>> ref_transaction_create|update|delete()*
>> ref_transaction_commit()
>>
>> but, if a caller such as walker_fetch() could opt to do
>> ref_transaction_begin()
>> ref_transaction_lock_ref()*
>> ...do stuff...
>> ref_transaction_create|update|delete()*
>> ref_transaction_commit()
>>
>> In this second case ref_transaction_commit() would only take out any locks
>> that
>> are missing during the 'lock the refs" loop.
>>
>> Suggestion 1: Add a ref_transaction_lock_ref() to allow locking a ref
>> early during
>> a transaction.
>
> Hmph.
>
> I am not sure if that is the right way to go, or instead change all
> create/update/delete to take locks without adding a new primitive.
Junio's suggestion seems like a good idea to me. Obviously, as soon as
we take out a lock we could also do any applicable old_sha1 check and
possibly fail fast.
Does a "verify" operation require holding a lock? If not, when is the
verification done--early, or during the commit, or both? (I realize
that we don't yet *have* a verify operation at the API level, but we
should!)
We also need to think about for what period of time we have to hold the
packed-refs lock.
Finally, we shouldn't forget that currently the reflog files are locked
by holding the lock on the corresponding loose reference file. Do we
need to integrate these files into our system any more than they
currently are?
[By the way, I noticed the other day that the command
git reflog expire --stale-fix --expire-unreachable=now --all
can hold loose reference locks for a *long* time (like 10s of minutes),
especially in weird cases like the repository having 9000 packfiles for
some reason or another :-) The command execution time grows strongly
with the length of the reference's log, or maybe even the square of the
length assuming the history is roughly linear and reachability is
computed separately for each SHA-1. This is just an empirical
observation so far; I haven't looked into the code yet.]
>> A second idea is to change the signatures for
>> ref_transaction_create|update|delete()
>> slightly and allow them to return errors early.
>> We can check for things like add_update() failing, check that the
>> ref-name looks sane,
>> check some of the flags, like if has_old==true then old sha1 should
>> not be NULL or 0{40}, etc.
>>
>> Additionally for robustness, if any of these functions detect an error
>> we can flag this in the
>> transaction structure and take action during ref_transaction_commit().
>> I.e. if a ref_transaction_update had a hard failure, do not allow
>> ref_transaction_commit()
>> to succeed.
>>
>> Suggestion 2: Change ref_transaction_create|delete|update() to return an int.
>> All callers that use these functions should check the function for error.
>
> I think that is a very sensible thing to do.
>
> The details of determining "this cannot possibly succeed" may change
> (for example, if we have them take locks at the point of
> create/delete/update, a failure to lock may count as an early
> error).
>
> Is there any reason why this should be conditional (i.e. you said
> "allow them to", implying that the early failure is optional)?
Also a good suggestion. We should make it clear in the documentation
that the create/delete/update functions are not *obliged* to return an
error (for example that the current value of the reference does not
agree with old_sha1) because future alternate ref backends might
possibly not be able to make such checks until the commit phase. For
example, checking old_sha1 might involve a round-trip to a database or
remote repository, in which case it might be preferable to check all
values in a single round-trip upon commit.
So, callers might be informed early of problems, or they might only
learn about problems when they try to commit the transaction. They have
to be able to handle either type of error reporting.
So then the question arises (and maybe this is what Ronnie was getting
at by suggesting that the checks might be conditional): are callers
*obliged* to check the return values from create/delete/update, or are
they allowed to just keep throwing everything into the transaction,
ignoring errors, and only check the result of ref_transaction_commit()?
I don't feel strongly one way or the other about this question. It
might be nice to be able to write callers sloppily, but it would cost a
bit more code in the reference backends. Though maybe it wouldn't even
be much extra code, given that we would probably want to put consistency
checks in there anyway.
>> Suggestion 3: remove the qsort and check for duplicates in
>> ref_transact
hhh
发自我的 iPad
Re: [PATCH] tag: add -i and --introduced modifier for --contains
"Luis R. Rodriguez" writes: > From: "Luis R. Rodriguez" > > Upstream Linux kernel commit c5905afb was introduced on v3.4 but > git describe --contains yields v3.5 Actually, "describe --contains" should yield v3.5-rc1~120^3~76^2, not v3.5. And you are right that the commit is contained in v3.4, so we also should be able to describe it as v3.4~479^2~9^2 as well. And between v3.4 and v3.5-rc1, the latter is a closer anchor point for that commit (v3.5-rc1 only needs about 200 hops to reach the commit, while from v3.4 you would need close to 500 hops), hence we end up picking the latter as "a better answer". Now, with the explanation of how/why this happens behind us, I see two possible issues with this patch: - The reason a human-user rejects v3.5-rc1~120^3~76^2 as the solution and favor v3.4~479^2~9^2 could be because of the -rc1 part in the answer. Perhaps we would want an option that affects which tags are to be used (and which tags are to be excluded) as anchoring points? - If we are truly interested in finding out the "earliest tag that contains the given commit", shouldn't we be ignoring the tagname and go with the tag with the oldest timestamp? After all, there may be a fix merged to v7.0 first on April 1st, and then on a later date the same fix may be merged to the maintenance track to be tagged as v6.9.1 on May 5th, and in such a case, wouldn't you want to say that the fix first appeared on v7.0 on April 1st, instead of on May 5th? 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 v2 1/2] sequencer.c: check for lock failure and bail early in fast_forward_to
On 04/16/2014 08:56 PM, Ronnie Sahlberg wrote:
> Change fast_forward_to() to check if locking the ref failed, print a nice
> error message and bail out early.
> The old code did not check if ref_lock was NULL and relied on the fact
> that the write_ref_sha1() would safely detect this condition and set the
s/the write_ref_sha1()/write_ref_sha1()/
> return variable ret to indicate an error.
> While that is safe, it makes the code harder to read for two reasons:
> * Inconsistency. Almost all other places we do check the lock for NULL
> explicitely, so the naive reader is confused "why don't we check here".
s/explicitely/explicitly/
s/here"/here?"/
> * And relying on write_ref_sha1() to detect and return an error for when
> a previous lock_any_ref_for_update() feels obfuscated.
s/feels/failed feels/ maybe?
>
> This change should not change any functionality or logic
> aside from adding an extra error message when this condition is triggered.
> (write_ref_sha1() returns an error silently for this condition)
You need a period inside the parentheses.
>
> Signed-off-by: Ronnie Sahlberg
> ---
> sequencer.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/sequencer.c b/sequencer.c
> index bde5f04..0a80c58 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -281,8 +281,12 @@ static int fast_forward_to(const unsigned char *to,
> const unsigned char *from,
> exit(1); /* the callee should have complained already */
> ref_lock = lock_any_ref_for_update("HEAD", unborn ? null_sha1 : from,
> 0, NULL);
> + if (!ref_lock)
> + return error(_("Failed to lock HEAD during fast_forward_to"));
This error message can be emitted to the user in the normal course of
things (i.e., it is not a bug). So the message should make sense to the
user. Is "fast_forward_to" a user-facing term that the user will
understand? I suspect that you took it from the name of the function,
which is *not* meaningful to a user.
But unfortunately I'm not familiar enough with the sequencer to be able
to suggest a better error message.
> +
> strbuf_addf(&sb, "%s: fast-forward", action_name(opts));
> ret = write_ref_sha1(ref_lock, to, sb.buf);
> +
> strbuf_release(&sb);
> return ret;
> }
>
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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/2] commit.c: check for lock error and return early
On 04/16/2014 08:56 PM, Ronnie Sahlberg wrote:
> Move the check for the lock failure to happen immediately after
> lock_any_ref_for_update().
> Previously the lock and the check-if-lock-failed was separated by a handful
> of string manipulation statements.
Please flow sentences together into paragraphs for easier reading,
rather than having an extremely ragged right-hand margin.
The rest looks good.
Michael
>
> Moving the check to occur immediately after the failed lock makes the
> code slightly easier to read and makes it follow the pattern of
> try-to-take-a-lock()
> if (check-if-lock-failed){
> error
> }
> ---
> builtin/commit.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index d9550c5..c6320f1 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1672,6 +1672,10 @@ int cmd_commit(int argc, const char **argv, const char
> *prefix)
> ? NULL
> : current_head->object.sha1,
> 0, NULL);
> + if (!ref_lock) {
> + rollback_index_files();
> + die(_("cannot lock HEAD ref"));
> + }
>
> nl = strchr(sb.buf, '\n');
> if (nl)
> @@ -1681,10 +1685,6 @@ int cmd_commit(int argc, const char **argv, const char
> *prefix)
> strbuf_insert(&sb, 0, reflog_msg, strlen(reflog_msg));
> strbuf_insert(&sb, strlen(reflog_msg), ": ", 2);
>
> - if (!ref_lock) {
> - rollback_index_files();
> - die(_("cannot lock HEAD ref"));
> - }
> if (write_ref_sha1(ref_lock, sha1, sb.buf) < 0) {
> rollback_index_files();
> die(_("cannot update HEAD ref"));
>
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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] tag: add -i and --introduced modifier for --contains
On Wed, Apr 16, 2014 at 3:02 PM, Junio C Hamano wrote: > "Luis R. Rodriguez" writes: > >> From: "Luis R. Rodriguez" >> >> Upstream Linux kernel commit c5905afb was introduced on v3.4 but >> git describe --contains yields v3.5 > > Actually, "describe --contains" should yield v3.5-rc1~120^3~76^2, > not v3.5. Yes, indeed thanks, sorry I should have been explicit. > And you are right that the commit is contained in v3.4, so we also > should be able to describe it as v3.4~479^2~9^2 as well. That'd be swell :) > And between v3.4 and v3.5-rc1, the latter is a closer anchor point > for that commit (v3.5-rc1 only needs about 200 hops to reach the > commit, while from v3.4 you would need close to 500 hops), Ah! Thanks for explaining this mysterious puzzle to me. I'm a bit perplexed why still. Can I trouble you for a little elaboration here? How could one view from a commit merged on v3.4 possibly yield more commits to v3.4 than to v3.5 ? Is it because it starts counting on the merge's parent (v3.3) ? > hence we > end up picking the latter as "a better answer". > > Now, with the explanation of how/why this happens behind us, I see > two possible issues with this patch: > > - The reason a human-user rejects v3.5-rc1~120^3~76^2 as the >solution and favor v3.4~479^2~9^2 could be because of the -rc1 >part in the answer. Perhaps we would want an option that affects >which tags are to be used (and which tags are to be excluded) as >anchoring points? I'd take an rc release as a blessed point too so not sure, and come to think of it I'm not a bit perplexed why the results for my change did not yield an rc1 as well. > - If we are truly interested in finding out the "earliest tag that >contains the given commit", shouldn't we be ignoring the tagname >and go with the tag with the oldest timestamp? After all, there >may be a fix merged to v7.0 first on April 1st, and then on a >later date the same fix may be merged to the maintenance track to >be tagged as v6.9.1 on May 5th, At least for Linux linux-3.X.y branches (one example linux-3.4.y) on linux-stable has different commit IDs from patches cherry picked from Linus' tree, and that patch just referneces the upstream commit from Linus' tree on the commit log, but nothing more. > and in such a case, wouldn't you want to say that the fix first appeared on > v7.0 on April 1st, > instead of on May 5th? Sure, but I'd expect the folks maintaining v6.9.x would just refer to the upstream commit ID from v7.0. Luis -- 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
[l10n] date: Note for translators not included in .po files
A note for translators in date.c is not included in git.pot. Namely, the following note from date.c:147 is not included (https://github.com/git/git/blob/v1.9.2/date.c#L147): /* TRANSLATORS: "%s" is " years" */ This is a very useful note for translators (in fact, I think the zh_CN translation for date.c:149 might be a little off because this note was not included. My Mandarin is rusty, but I believe " years, months ago" should be expressed without a comma). According to po/README, the l10n coordinator is responsible for updating the git.pot file. Would it be possible to update it based on v1.9.2 and include the above comment? By the way, I am trying to organize contributors to produce a Japanese localization for Core Git. Currently we have plenty of interest but only two contributors. If you or anyone you know would like to contribute please visit the repository here: https://github.com/modocache/git-po-ja Thanks! - Brian Gesiak -- 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: gitignore vs. exclude vs assume-unchanged?
On 2014-04-16 10:51, Junio C Hamano wrote: a...@bellandwhistle.net writes: Any clarification on the differences much appreciated: http://stackoverflow.com/questions/23097368/git-ignore-vs-exclude-vs-assume-unchanged/23097509 Please don't force people to refer to external site. The .gitignore and .git/info/exclude are the two UIs to invoke the same mechanism. In-tree .gitignore are to be shared among project members (i.e. everybody working on the project should consider the paths that match the ignore pattern in there as cruft). On the other hand, .git/info/exclude is meant for personal ignore patterns (i.e. you, while working on the project, consider them as cruft). Assume-unchanged should not be abused for an ignore mechanism. It is "I know my filesystem operations are slow. I'll promise Git that I won't change these paths by making them with that bit---that way, Git does not have to check if I changed things in there every time I ask for 'git status' output". It does not mean anything other than that. Especially, it is *not* a promise by Git that Git will always consider these paths are unmodified---if Git can determine a path that is marked as assume-unchanged has changed without incurring extra lstat(2) cost, it reserves the right to report that the path *has been* modified (as a result, "git commit -a" is free to commit that change). Thanks June. Great to hear this authoritatively. IMHO your very helpful explanation about typical use cases, the purpose of 'exclude, and assume-unchanged not being a "promise" is missing from the docs, or at least not obviously present: http://git-scm.com/docs In particular, 'exclude' is spottily documented. I realize the docs are structured strictly as an API reference, but it would be great to see a comparison of ignore techniques spelled out. FWIW I asked several people I think of as experts and none of them felt sure of their answer. :) thanks again. -- 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] git-rebase: Print name of rev when using shorthand
> The concept of "n-th prior checkout" (aka @{-n}) and "immediately
> previous checkout" (aka "-") are equivalent, even though the former
> may be more generic.
>
> You seem to be saying that those who understand the former are with
> superiour mental capacity in general than those who only know the
> latter, and they can always remember where they came from.
>
> ...have you considered _why_ it is a good thing to show the symbolic
> name in the first place?
I think I failed to express my point here; I don't think people that
use "@{-1}" have superior mental capacity, but rather simply that
they are aware of the "@{-n}" method of specifying a previous reference.
So in response to the command "git rebase @{-4}", displaying the
result "Fast-forwarded HEAD to @{-4}" does not contain any unknown
syntax that may confuse them. They may not remember what "@{-4}"
refers to, but they are aware of the syntax at least.
On the other hand, people who use the "-" shorthand may or may
not be aware of the "@{-n}" syntax. In that respect, I think it would
be confusing to display "Fast-forwarded HEAD to @{-1}" in response
to the command "git rebase -"; the user may not know what "@{-1}"
means!
Thus my original point was that I felt displaying a symbolic name in
response to "git rebase -" was more important than doing so in
response to "git rebase @{-1}". The issue isn't about forgetting what
"@{-n}" refers to, it's whether the user even knows what "@{-n}" is
supposed to mean.
But in light of your other comments:
>> Furthermore, were we to translate "@{-1}", does that mean we
>> should also translate "@{-2}" or prior?
>
> Surely, why not. If a user is so forgetful to need help remembering
> where s/he was immediately before, wouldn't it be more helpful to
> give "here is where you were" reminder for older ones to allow them
> to double check they specified the right thing and spot possible
> mistakes?
>
> ...
>
> Giving the symbolic name 'master' is good because it is possible
> that the user thought the previous branch was 'frotz', forgetting
> that another branch was checked out tentatively in between, and the
> user ended up rebasing on top of a wrong branch. Telling what that
> previous branch is is a way to help user spot such a potential
> mistake. So I am all for making "rebase -" report what concrete
> branch the branch was replayed on top of, and consider it an incomplete
> improvement if "rebase @{-1}" (or "rebase @{-2}") did not get the
> same help---especially when I know that the underlying mechanism you
> would use to translate @{-1} back to the concrete branch name is the
> same for both cases anyway.
I had not originally thought of this, perhaps because I was preoccupied
with preventing users from seeing syntax they might not be aware of.
But I definitely agree that displaying symbolic names for all "@{-n}"
is a good way to prevent user error.
> I can buy "that would be a lot more work, and I do not want to do it
> (or I do not think I can solve it in a more general way)", though.
Perish the thought! :)
I will try to re-roll this patch to include symbolic names for "@{-n}".
As usual, thanks for the feedback!
- Brian Gesiak
--
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: gitignore vs. exclude vs assume-unchanged?
Hi, a...@bellandwhistle.net wrote: > In particular, 'exclude' is spottily documented. Where did you expect to read about it? I see some mention of .git/info/exclude in the gitignore(5) page, but I wouldn't be surprised if there's room for improvement there (improvements welcome). > I realize the docs > are structured strictly as an API reference, No, the docs are meant to be helpful, not just to confirm what people already know. ;-) Thanks, Jonathan -- 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 1/3] rebase: avoid non-function use of "return" on FreeBSD
On Apr 16, 2014, at 11:11, Junio C Hamano wrote:
Junio C Hamano writes:
"Kyle J. McKay" writes:
If I'm the only one getting a wrong meaning from the comments,
then no
reason to change them.
I agree that the description does not read well with the work-around
already there. I am not sure what would be a better way to phrase
it, though.
Here is a tentative rewrite at the beginning and the end of rebase-am:
# The whole contents of the file is run by dot-sourcing this
# file from inside a shell function. It used to be that
# "return"s we see below were not inside any function, and
# expected to return from the function that dot-sourced us.
I think it's the "return from the function that dot-sourced us" that
gives me the wrong meaning. The "return from" part says to me that
function will be returning which it will not unless the dot command
was the last command in the function.
Either "return to the function that dot-sourced us" or "return from
the dot command that dot-sourced us", but using the original wording
implies to me that the function that dot-sourced us will return as
soon as the dot-sourced script executes the return and that is exactly
one of the bugs we're working around.
I think just the s/from/to/ would fix it so it does not give me the
wrong impression, but that doesn't mean that would not confuse
everyone else. ;)
#
# However, FreeBSD /bin/sh misbehaves on such a construct and
# continues to run the statements that follow such a "return".
# As a work-around, we introduce an extra layer of a function
# here, and immediately call it after defining it.
git_rebase__am () {
...
}
# ... and then we call the whole thing.
git_rebase__am
On Apr 16, 2014, at 11:23, Junio C Hamano wrote:
Junio C Hamano writes:
By the way, you have this in your log message:
... the git-rebase--*.sh scripts have used a "return" to return
from the "dot" command that runs them. While this is allowed by
POSIX,...
Is it "this is allowed", or is it "this should be the way and shells
that do not do so are buggy"?
Answering myself...
The only "unspecified" I see is this:
If the shell is not currently executing a function or dot
script, the results are unspecified.
which clearly does not apply to the version before this patch (we
are executing a dot script). And
The return utility shall cause the shell to stop executing the
current function or dot script.
would mean that we are correct to expect that "should not get here"
is not reached, as the "return 5" would cause the shell to stop
executing the dot script there.
So "while this is allowed by POSIX" may be a bit misleading and
needs to be reworded, I guess?
"allowed by POSIX" is not incorrect. ;)
But perhaps the log message should say 'While POSIX specifies that a
"return" may be used to exit a "dot" sourced script,' there instead to
be clearer. Which would make that whole first paragraph become:
Since a1549e10, 15d4bf2e and 01a1e646 (first appearing in v1.8.4)
the
git-rebase--*.sh scripts have used a "return" to return from the
"dot"
command that runs them. While POSIX specifies that a "return"
may be
used to exit a "dot" sourced script, the FreeBSD /bin/sh utility
behaves poorly under some circumstances when such a "return" is
executed.
--
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: [tig] [PATCHv2 3/3] log: Colour the diff stat
On Sun, Apr 13, 2014 at 5:54 PM, Kumar Appaiah
wrote:
>
> This commit adds custom log_read and log_draw functions that utilize
> the diff stat drawing functions from the diff module. The absence of
> the triple hyphen separator prevents direct usage of the diff drawing
> functions directly.
See my comments below.
> ---
> src/log.c | 55 +--
> 1 file changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/src/log.c b/src/log.c
> index 40c9a21..468f7c3 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -23,6 +23,9 @@ struct log_state {
> * up/down in the log view. */
> int last_lineno;
> enum line_type last_type;
> + bool commit_title_read;
> + bool after_commit_header;
> + bool reading_diff_stat;
> };
>
> static void
> @@ -78,14 +81,62 @@ log_request(struct view *view, enum request request,
> struct line *line)
> }
> }
>
> +static bool
> +log_read(struct view *view, char *data)
> +{
> + enum line_type type;
> + struct log_state *state = view->private;
> + size_t len;
> +
> + if (!data)
> + return TRUE;
> +
> + type = get_line_type(data);
> + len = strlen(data);
> +
> + if (type == LINE_COMMIT)
> + state->commit_title_read = TRUE;
> + else if (state->commit_title_read && len < 1) {
> + state->commit_title_read = FALSE;
> + state->after_commit_header = TRUE;
> + } else if (state->after_commit_header && len < 1) {
> + state->after_commit_header = FALSE;
> + state->reading_diff_stat = TRUE;
> + } else if (state->reading_diff_stat) {
> + bool ret = diff_common_add_diff_stat(view, data);
> + if (ret) {
> + return TRUE;
> + } else {
> + state->reading_diff_stat = FALSE;
> + }
> + }
> +
> + return pager_common_read(view, data, type);
> +}
> +
> +static bool
> +log_draw(struct view *view, struct line *line, unsigned int lineno)
> +{
> + char *text = line->data;
> + enum line_type type = line->type;
> +
This is missing a call to draw_lineno(...)
> + if (type == LINE_DIFF_STAT) {
> + diff_common_draw_diff_stat(view, &type, &text);
> + draw_text(view, type, text);
I had to #include "tig/draw.h" for this to compile.
> + return TRUE;
> + }
> +
> + return pager_draw(view, line, lineno);
> +}
> +
> static struct view_ops log_ops = {
> "line",
> argv_env.head,
> VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER |
> VIEW_LOG_LIKE | VIEW_REFRESH,
> sizeof(struct log_state),
> log_open,
> - pager_read,
> - pager_draw,
> + log_read,
> + log_draw,
> log_request,
> pager_grep,
> log_select,
> --
> 1.9.1
>
--
Jonas Fonseca
--
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: [tig] [PATCHv2 0/3] log: colour the diffstat
Hi Kumar, On Sun, Apr 13, 2014 at 5:54 PM, Kumar Appaiah wrote: > > These patches add colourization to the log view. They reuse the diff > stat drawing functions from the diff module directly. This is a great idea. I wonder though if it would make sense to put this into the pager backend instead so all pager based views can benefit unless of course the "state machine" would end up becoming too complicated. > This version just includes some code reformatting and minor > fixes. Please comment on what other fixes could help. See my other email regarding fixes. Since I am currently refactoring how views are drawn I'd prefer to postpone merging this patchset until the dust has settled. I'll try to rebase the patches once I get there before reaching out to you. -- Jonas Fonseca -- 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
LOAN OFFER
Dear valued customer, Do you need an urgent loan to pay of your bills, invest more on your business, if yes PREMIUM CAPITAL LOAN offer loan at 3% interest rate. We are fast and reliable when it comes to loan lending contact email: premiumcapitall...@hotmail.co.uk for more information. Contact email: premiumcapitall...@hotmail.co.uk Best Regard PAMELA MARTHA Loan officer Mansfield Road Rotherham South Yorkshire S70 2EB email: premiumcapitall...@hotmail.co.uk Tell: +447035958575 Fax: +448447742385 -- 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: [tig] [PATCHv2 3/3] log: Colour the diff stat
On Wed, Apr 16, 2014 at 08:44:41PM -0400, Jonas Fonseca wrote:
> On Sun, Apr 13, 2014 at 5:54 PM, Kumar Appaiah
> wrote:
> >
> > This commit adds custom log_read and log_draw functions that utilize
> > the diff stat drawing functions from the diff module. The absence of
> > the triple hyphen separator prevents direct usage of the diff drawing
> > functions directly.
>
> See my comments below.
Hi Jonas.
> > +static bool
> > +log_draw(struct view *view, struct line *line, unsigned int lineno)
> > +{
> > + char *text = line->data;
> > + enum line_type type = line->type;
> > +
>
> This is missing a call to draw_lineno(...)
Noted.
> > + if (type == LINE_DIFF_STAT) {
> > + diff_common_draw_diff_stat(view, &type, &text);
> > + draw_text(view, type, text);
>
> I had to #include "tig/draw.h" for this to compile.
I'll take care of this.
I'll send you a pull request eventually. You can handle it after your
refactor is complete.
Thanks!
Kumar
--
Kumar Appaiah
--
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: [l10n] date: Note for translators not included in .po files
2014-04-17 6:51 GMT+08:00 Brian Gesiak :
> A note for translators in date.c is not included in git.pot.
> Namely, the following note from date.c:147 is not included
> (https://github.com/git/git/blob/v1.9.2/date.c#L147):
>
> /* TRANSLATORS: "%s" is " years" */
>
Comments for translators will be extracted to pot file automatically,
when run xgettext with "--add-comments" option.
-c, --add-comments
place all comment blocks preceding keyword lines in output file
For example, the comments in the following code blocks will
be extracted.
/* TRANSLATORS: "will be extracted." */
strbuf_addf(&sb, Q_("%lu year", "%lu years", years), years);
strbuf_addf(&sb,
/* TRANSLATORS: "will be extracted." */
Q_("%lu year", "%lu years", years), years);
But if the comment is not right before the l10n markers, such
comments will not be extracted. E.g.
/* TRANSLATORS: "WARNING: will NOT be extracted." */
strbuf_addf(&sb,
Q_("%lu year", "%lu years", years), years);
/* TRANSLATORS: "WARNING: will NOT be extracted." */
strbuf_addf(&sb, Q_(
"%lu year", "%lu years", years), years);
I will scan all the codes and make a fix.
> This is a very useful note for translators (in fact, I think
> the zh_CN translation for date.c:149 might be a little off
> because this note was not included. My Mandarin is rusty,
> but I believe " years, months ago" should be expressed
> without a comma).
>
> According to po/README, the l10n coordinator is responsible
> for updating the git.pot file. Would it be possible to update it based
> on v1.9.2 and include the above comment?
>
I could generate a new git.pot for "maint" branch, but fixes for codes
may only contribute to "master" branch.
--
Jiang Xin
--
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] send-email: recognize absolute path on Windows
On Wed, Apr 16, 2014 at 10:19:54AM -0700, Junio C Hamano wrote:
> Ahh, OK, if you did so, you won't have any place to hook the "only
> on msys do this" trick into.
>
> It somehow feels somewhat confusing that we define a sub with the
> same name as the system one, while not overriding it entirely but
> delegate back to the system one. I am debating myself if it is more
> obvious if it is done this way:
>
> use File::Spec::Functions qw(file_name_is_absolute);
> if ($^O eq 'msys') {
You would probably want a "no warnings 'redefine'" here as well.
> sub file_name_is_absolute {
> return $_[0] =~ /^\// || $_[0] =~ /^[A-Z]:/i;
> }
> }
--
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] Update SVN.pm
Hello,
On Wed, Apr 16, 2014 at 12:13:21PM -0700, Junio C Hamano wrote:
> Interesting. What other strange forms can they record in their
> repositories, I have to wonder. Can they do
> 2014-01-07T5:8:6.048176Z
> for example?
Roman Belinsky, the author of this fix, witnessed after large scale
conversion that the problem happens with the hour part only.
(SVN commits from the same origin did this with hours but not with
minutes.) Recorded here:
https://github.com/msysgit/git/pull/126#discussion_r9661916
> I am wondering if it is simpler and less error prone
> to turn all these "we only accept two digits" into "\d+" not only
> for the hour part but also minute and second parts.
But Roman's proposed regexp nicely shows 1) what the standard is and
2) what is the deviation.
Have a nice day,
Stepan Kasal
> > Signed-off-by: Stepan Kasal
> > ---
> > perl/Git/SVN.pm | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
> > index a59564f..09cff13 100644
> > --- a/perl/Git/SVN.pm
> > +++ b/perl/Git/SVN.pm
> > @@ -1321,7 +1321,7 @@ sub get_untracked {
> > sub parse_svn_date {
> > my $date = shift || return '+ 1970-01-01 00:00:00';
> > my ($Y,$m,$d,$H,$M,$S) = ($date =~ /^(\d{4})\-(\d\d)\-(\d\d)T
> > - (\d\d)\:(\d\d)\:(\d\d)\.\d*Z$/x) or
> > + (\d\d?)\:(\d\d)\:(\d\d)\.\d*Z$/x) or
> > croak "Unable to parse date: $date\n";
> > my $parsed_date;# Set next.
> --
> 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
--
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/3] extract proper comments for l10n translators
When generate git.pot, many irrelevant comments are extracted as references for translators, but one useful comment is lost. This series patches will fix this issue. Jiang Xin (3): i18n: Fixes uncatchable comments for translators i18n: Only extract comments marked by special tag i18n: Remove obsolete comments for translators Makefile | 2 +- builtin/init-db.c | 8 +++- date.c| 2 +- diff.c| 8 4 files changed, 5 insertions(+), 15 deletions(-) -- 1.9.2.461.g942803f -- 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/3] i18n: Fixes uncatchable comments for translators
Comment for l10n translators can not be extracted by xgettext if it is
not right above the l10n tag. Moving the comment right before the l10n
tag will fix this issue.
Reported-by: Brian Gesiak
Signed-off-by: Jiang Xin
---
date.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/date.c b/date.c
index e1a2cee..782de95 100644
--- a/date.c
+++ b/date.c
@@ -144,8 +144,8 @@ void show_date_relative(unsigned long time, int tz,
if (months) {
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, Q_("%lu year", "%lu years", years),
years);
- /* TRANSLATORS: "%s" is " years" */
strbuf_addf(timebuf,
+/* TRANSLATORS: "%s" is " years" */
Q_("%s, %lu month ago", "%s, %lu months ago",
months),
sb.buf, months);
strbuf_release(&sb);
--
1.9.2.461.g942803f
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] i18n: Remove obsolete comments for translators
Since we do not translate diffstat any more, remove the obsolete comments.
Signed-off-by: Jiang Xin
---
diff.c | 8
1 file changed, 8 deletions(-)
diff --git a/diff.c b/diff.c
index 539997f..54d5308 100644
--- a/diff.c
+++ b/diff.c
@@ -1461,20 +1461,12 @@ int print_stat_summary(FILE *fp, int files, int
insertions, int deletions)
* but nothing about added/removed lines? Is this a bug in Git?").
*/
if (insertions || deletions == 0) {
- /*
-* TRANSLATORS: "+" in (+) is a line addition marker;
-* do not translate it.
-*/
strbuf_addf(&sb,
(insertions == 1) ? ", %d insertion(+)" : ", %d
insertions(+)",
insertions);
}
if (deletions || insertions == 0) {
- /*
-* TRANSLATORS: "-" in (-) is a line removal marker;
-* do not translate it.
-*/
strbuf_addf(&sb,
(deletions == 1) ? ", %d deletion(-)" : ", %d
deletions(-)",
deletions);
--
1.9.2.461.g942803f
--
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/3] i18n: Only extract comments marked by special tag
When extract l10n messages, we use "--add-comments" option to keep
comments right above the l10n messages for references. But sometimes
irrelevant comments are also extracted. For example in the following
code block, the comment in line 2 will be extracted as comment for the
l10n message in line 3, but obviously it's wrong.
{ OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
NULL /* takes no arguments */,
N_("ignore paths removed in the working tree (same as
--no-all)"),
PARSE_OPT_NOARG, ignore_removal_cb },
Since almost all comments for l10n translators are marked with the same
prefix (tag): "TRANSLATORS:", it's safe to only extract comments with
this special tag. I.E. it's better to call xgettext as:
xgettext --add-comments=TRANSLATORS: ...
Also tweaks the multi-line comment in "init-db.c", to make it start with
the proper tag, not "* TRANSLATORS:" (which has a star before the tag).
Signed-off-by: Jiang Xin
---
Makefile | 2 +-
builtin/init-db.c | 8 +++-
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/Makefile b/Makefile
index 2128ce3..a53f3a8 100644
--- a/Makefile
+++ b/Makefile
@@ -2102,7 +2102,7 @@ pdf:
XGETTEXT_FLAGS = \
--force-po \
- --add-comments \
+ --add-comments=TRANSLATORS: \
--msgid-bugs-address="Git Mailing List " \
--from-code=UTF-8
XGETTEXT_FLAGS_C = $(XGETTEXT_FLAGS) --language=C \
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c7c76bb..56f85e2 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -412,11 +412,9 @@ int init_db(const char *template_dir, unsigned int flags)
if (!(flags & INIT_DB_QUIET)) {
int len = strlen(git_dir);
- /*
-* TRANSLATORS: The first '%s' is either "Reinitialized
-* existing" or "Initialized empty", the second " shared" or
-* "", and the last '%s%s' is the verbatim directory name.
-*/
+ /* TRANSLATORS: The first '%s' is either "Reinitialized
+ existing" or "Initialized empty", the second " shared" or
+ "", and the last '%s%s' is the verbatim directory name. */
printf(_("%s%s Git repository in %s%s\n"),
reinit ? _("Reinitialized existing") : _("Initialized
empty"),
shared_repository ? _(" shared") : "",
--
1.9.2.461.g942803f
--
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/3] extract proper comments for l10n translators
2014-04-17 13:37 GMT+08:00 Jiang Xin : > When generate git.pot, many irrelevant comments are extracted as references > for translators, but one useful comment is lost. This series patches will > fix this issue. Brief changes of po/git.pot after applied these patches: diff --git a/po/git.pot b/po/git.pot index f70b46b..9e474f3 100644 --- a/po/git.pot +++ b/po/git.pot @@ -23,10 +23,6 @@ msgstr "" msgid "hint: %.*s\n" msgstr "" -#. -#. * Message used both when 'git commit' fails and when -#. * other commands doing a merge do. -#. #: advice.c:85 msgid "" "Fix them up in the work tree,\n" @@ -399,6 +395,7 @@ msgid_plural "%lu years" msgstr[0] "" msgstr[1] "" +#. TRANSLATORS: "%s" is " years" #: date.c:149 #, c-format msgid "%s, %lu month ago" @@ -582,8 +579,6 @@ msgstr "" msgid "Removing %s to make room for subdirectory\n" msgstr "" -#. something else exists -#. .. but not some other error (who really cares what?) #: merge-recursive.c:700 merge-recursive.c:721 msgid ": perhaps a D/F conflict?" msgstr "" @@ -899,11 +894,6 @@ msgstr "" msgid "Pathspec '%s' is in submodule '%.*s'" msgstr "" -#. -#. * We may want to substitute "this command" with a command -#. * name. E.g. when add--interactive dies when running -#. * "checkout -p" -#. #: pathspec.c:353 #, c-format msgid "%s: pathspec magic not supported by this command: %s" @@ -953,11 +943,6 @@ msgstr "" msgid "%s tracks both %s and %s" msgstr "" -#. -#. * This last possibility doesn't occur because -#. * FETCH_HEAD_IGNORE entries always appear at -#. * the end of the list. -#. #: remote.c:774 msgid "Internal error" msgstr "" @@ -1306,13 +1291,11 @@ msgstr "" msgid "Could not find section in .gitmodules where path=%s" msgstr "" -#. Maybe the user already did that, don't error out here #: submodule.c:76 #, c-format msgid "Could not update .gitmodules entry %s" msgstr "" -#. Maybe the user already did that, don't error out here #: submodule.c:109 #, c-format msgid "Could not remove .gitmodules entry for %s" @@ -1884,7 +1867,6 @@ msgstr "" msgid "add changes from all tracked and untracked files" msgstr "" -#. takes no arguments #: builtin/add.c:260 msgid "ignore paths removed in the working tree (same as --no-all)" msgstr "" @@ -2044,7 +2026,6 @@ msgstr "" msgid "corrupt binary patch at line %d: %.*s" msgstr "" -#. there has to be one hunk (forward hunk) #: builtin/apply.c:1900 #, c-format msgid "unrecognized binary patch at line %d" @@ -2232,7 +2213,6 @@ msgstr "" msgid "internal error" msgstr "" -#. Say this even without --verbose #: builtin/apply.c:4043 #, c-format msgid "Applying patch %%s with %d reject..." @@ -3232,7 +3212,6 @@ msgstr "" msgid " ... and %d more.\n" msgstr "" -#. The singular version #: builtin/checkout.c:711 #, c-format msgid "" @@ -3280,7 +3259,6 @@ msgstr "" msgid "invalid reference: %s" msgstr "" -#. case (1): want a tree #: builtin/checkout.c:1002 #, c-format msgid "reference is not a tree: %s" @@ -4276,7 +4254,6 @@ msgstr "" msgid "GPG sign commit" msgstr "" -#. end commit message options #: builtin/commit.c:1508 msgid "Commit contents options" msgstr "" @@ -5140,7 +5117,6 @@ msgstr "" msgid "See \"git help gc\" for manual housekeeping.\n" msgstr "" -#. be quiet on --auto #: builtin/gc.c:336 #, c-format msgid "" @@ -5894,12 +5870,10 @@ msgstr "" msgid "unable to move %s to %s" msgstr "" -#. -#. * TRANSLATORS: The first '%s' is either "Reinitialized -#. * existing" or "Initialized empty", the second " shared" or -#. * "", and the last '%s%s' is the verbatim directory name. -#. -#: builtin/init-db.c:420 +#. TRANSLATORS: The first '%s' is either "Reinitialized +#. existing" or "Initialized empty", the second " shared" or +#. "", and the last '%s%s' is the verbatim directory name. +#: builtin/init-db.c:418 #, c-format msgid "%s%s Git repository in %s%s\n" msgstr "" @@ -6627,7 +6601,6 @@ msgstr "" msgid "Commit %s has a bad GPG signature allegedly by %s." msgstr "" -#. 'N' #: builtin/merge.c:1279 #, c-format msgid "Commit %s does not have a GPG signature." @@ -9593,8 +9566,6 @@ msgstr "" msgid "'git bisect bad' can take only one argument." msgstr "" -#. have bad but not good. we could bisect although -#. this is less optimum. #: git-bisect.sh:273 msgid "Warning: bisecting only with a bad commit." msgstr "" @@ -9690,10 +9661,6 @@ msgstr "" msgid "updating an unborn branch with changes added to the index" msgstr "" -#. The fetch involved updating the current branch. -#. The working tree and the index file is still based on the -#. $orig_head commit, but we are merging into $curr_head. -#. First update the working tree to match $curr_head. #: git-pull.sh:271 #, sh-format msgid "" @@ -9835,7 +9802,6 @@ msgstr "" msgid "Changes from $mb to $onto:" msgstr "" -#. Detach HEAD and reset the tree #: git-rebase.sh:609 msgid "First, rewinding head to replay your work on top of it..." msgstr "" @@ -10218,7 +10184,6 @@ msgstr "" msgid "The --
[RFC] Speed up "git status" by caching untracked file info
This patch serves as a heads up about a feature I'm working on. I hope
that by posting it early, people could double check if I have made
some fundamental mistakes that completely ruin the idea. It's about
speeding up "git status" by caching untracked file info in the index
_if_ your file system supports it (more below).
The whole WIP series is at
https://github.com/pclouds/git/commits/untracked-cache
I only post the real meat here. I'm aware of a few incomplete details
in this patch, but nothing fundamentally wrong. So far the numbers are
promising. ls-files is updated to run fill_directory() twice in a
row and "ls-files -o --directory --no-empty-directory --exclude-standard"
(with gcc -O0) gives me:
first run second (cached) run
gentoo-x86500 ms 71.6 ms
wine 140 ms 9.72 ms
webkit125 ms 6.88 ms
linux-2.6 106 ms 16.2 ms
Basically untracked time is cut to one tenth in the best case
scenario. The final numbers would be a bit higher because I haven't
stored or read the cache from index yet. Real commit message follows..
read_directory() plays a bit part in the slowness of "git status"
because it has to read every directory and check for excluded entries,
which is really expensive. This patch adds an option to cache the
results so that after the first slow read_directory(), the following
calls should be cheap and fast.
The following inputs are sufficient to determine what files in a
directory are excluded:
- The list of files and directories of the direction in question
- The $GIT_DIR/index
- The content of $GIT_DIR/info/exclude
- The content of core.excludesfile
- The content (or the lack) of .gitignore of all parent directories
from $GIT_WORK_TREE
If we can cheaply validate all those inputs for a certain directory,
we are sure that the current code will always produce the same
results, so we can cache and reuse those results.
This is not a silver bullet approach. When you compile a C file, for
example, the old .o file is removed and a new one with the same name
created, effectively invalidating the containing directory's
cache. But at least with a large enough work tree, there could be many
directories you never touch. The cache could help there.
The first input can be checked using directory mtime. In many
filesystems, directory mtime is updated when direct files/dirs are
added or removed (*). If you do not use such a file system, this
feature is not for you.
The second one can be hooked from read-cache.c. Whenever a file (or a
submodule) is added or removed from a directory, we invalidate that
directory. This will be done in a later patch.
The remaining inputs are easy, their SHA-1 could be used to verify
their contents. We do need to read .gitignore files and digest
them. But they are usually few and small, so the overhead should not
be much.
At the implementation level, the whole directory structure is saved,
each directory corresponds to one struct untracked_dir. Each directory
holds SHA-1 of the .gitignore underneath (or null if it does not
exist) and the list of untracked "files" and subdirs that need to
recurse into if all is well. Untracked subdirectories are saved in the
file queue and are the reason of quoting "files" in the previous
sentence.
On the first run, no untracked_dir is valid, the default code path is
run. prep_exclude() is updated to record SHA-1 of .gitignore along the
way. read_directory_recursive() is updated to record untracked files.
On subsequent runs, read_directory_recursive() reads stat info of the
directory in question and verifies if files/dirs have been added or
removed. With the help of prep_exclude() to verify .gitignore chain,
it may decide "all is well" and enable the fast path in
treat_path(). read_directory_recursive() is still called for
subdirectories even in fast path, because a directory mtime does not
cover all subdirs recursively.
So if all is really well, read_directory() becomes a series of
open(".gitignore"), read(".gitignore"), close(), hash_sha1_file() and
stat() _without_ heavyweight exclude filtering. There should be
no overhead if this feature is disabled.
(*) Looking at the code in linux-2.6, ext* family seems to expose this
behavior. vfat also does (but not so sure about fat). btrfs probably
does. statfs() could be used to detect file system.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
dir.c | 336 +-
dir.h | 31 ++
2 files changed, 343 insertions(+), 24 deletions(-)
diff --git a/dir.c b/dir.c
index bd58c14..f5d6315 100644
--- a/dir.c
+++ b/dir.c
@@ -31,8 +31,23 @@ enum path_treatment {
path_untracked
};
+struct cached_dir {
+ DIR *fdir;
+ struct untracked_dir *untracked;
+ int nr_files;
+ int nr_dirs;
+
+ /*
+* return data from read_cached_dir(). name and state are only
+* valid if de is NULL
+*/
+
Re: [l10n] date: Note for translators not included in .po files
2014-04-17 6:51 GMT+08:00 Brian Gesiak : > According to po/README, the l10n coordinator is responsible > for updating the git.pot file. Would it be possible to update it based > on v1.9.2 and include the above comment? > I have pushed a polished "git.pot" to the maint branch of git-l10n/git-po. It's based on Git v1.9.2, and you can get it from: https://github.com/git-l10n/git-po/blob/maint/po/git.pot But if you want to translate for the upcoming Git v2.0.0, you can use "git.pot" in the "pu" branch as a template. See: https://github.com/git-l10n/git-po/blob/pu/po/git.pot > By the way, I am trying to organize contributors to produce a Japanese > localization for Core Git. Currently we have plenty of interest but > only two contributors. If you or anyone you know would like to contribute > please visit the repository here: https://github.com/modocache/git-po-ja > > Thanks! > > - Brian Gesiak > -- > 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 -- 蒋鑫 北京群英汇信息技术有限公司 邮件: worldhello@gmail.com 网址: http://www.ossxp.com/ 博客: http://www.worldhello.net/ 微博: http://weibo.com/gotgit/ 电话: 18601196889 -- 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] Unicode: update of combining code points
On 16/04/2014 22:58, Torsten Bögershausen wrote: Excellent, thanks for the pointers. Running the script below shows that "0X00AD SOFT HYPHEN" should have zero length (and some others too). I wonder if that is really the case, and which one of the last 2 lines in the script is the right one. What does this mean for us: "CfFormat a format control character" Maybe dig back through the Git logs to check the original logic, but the comments suggest that "Cf" characters have been viewed as zero-width. That makes sense - they're usually markers indicating things like bidirectional text flow, so won't be taking space. (Although they may be causing even more extreme layout effects...) Soft-hyphen is noted as an explicit exception to the rule in the utf8.c comments. As of Unicode 4.0, it's supposed to be a character indicating a point where a hyphen could be placed if a line-wrap occurs, and if that wrap happens, then it can actually take up 1 space, otherwise not. So its width could be either 0 or 1, depending. Or, quite likely, the terminal doesn't treat it specially, and it always just looks like a hyphen... Thus we err on the safe side and give it width 1. See http://en.wikipedia.org/wiki/Soft_hyphen for background. The comments suggest adding "-00AD +1160-11FF" to the uniset command line for that tweak and for composing Hangul. (The +200B tweak isn't necessary any more - Zero-Width Space U+200B became Cf officially in Unicode 4.0.1: http://en.wikipedia.org/wiki/Zero-width_space http://www.unicode.org/review/resolved-pri.html#pri21 ) All of this is only really an approximation - a best-effort attempt to figure out the width of a string without any actual communication with the display device. So it'll never be perfect. The choice between double and single width in particular will often be unpredictable, unless you had deeper locale knowledge. Actually, while doing this, I've realised that this was originally Markus Kuhn's implementation, and that is acknowledged at the top of the file: http://www.cl.cam.ac.uk/~mgk25/ucs/wcwidth.c Good, because he knows what he's doing. Kevin -- 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/2] git-svn: only look at the new parts of svn:mergeinfo
In a Subversion repository where many feature branches are merged into a
trunk, the svn:mergeinfo property can grow very large. This severely
slows down git-svn's make_log_entry() because it is checking all
mergeinfo entries every time the property changes.
In most cases, the additions to svn:mergeinfo since the last commit are
pretty small, and there is nothing to gain by checking merges that were
already checked for the last commit in the branch.
Add a mergeinfo_changes() function which computes the set of interesting
changes to svn:mergeinfo since the last commit. Filter out merged
branches whose ranges haven't changed, and remove a common prefix of
ranges from other merged branches.
This speeds up "git svn fetch" by several orders of magnitude on a large
repository where thousands of feature branches have been merged.
Signed-off-by: Jakob Stoklund Olesen
---
perl/Git/SVN.pm | 84 -
1 file changed, 72 insertions(+), 12 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index a59564f..d3785ab 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1178,7 +1178,7 @@ sub find_parent_branch {
or die "SVN connection failed somewhere...\n";
}
print STDERR "Successfully followed parent\n" unless $::_q > 1;
- return $self->make_log_entry($rev, [$parent], $ed);
+ return $self->make_log_entry($rev, [$parent], $ed, $r0,
$branch_from);
}
return undef;
}
@@ -1210,7 +1210,7 @@ sub do_fetch {
unless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {
die "SVN connection failed somewhere...\n";
}
- $self->make_log_entry($rev, \@parents, $ed);
+ $self->make_log_entry($rev, \@parents, $ed, $last_rev);
}
sub mkemptydirs {
@@ -1478,9 +1478,9 @@ sub find_extra_svk_parents {
sub lookup_svn_merge {
my $uuid = shift;
my $url = shift;
- my $merge = shift;
+ my $source = shift;
+ my $revs = shift;
- my ($source, $revs) = split ":", $merge;
my $path = $source;
$path =~ s{^/}{};
my $gs = Git::SVN->find_by_url($url.$source, $url, $path);
@@ -1702,6 +1702,62 @@ sub parents_exclude {
return @excluded;
}
+# Compute what's new in svn:mergeinfo.
+sub mergeinfo_changes {
+ my ($self, $old_path, $old_rev, $path, $rev, $mergeinfo_prop) = @_;
+ my %minfo = map {split ":", $_ } split "\n", $mergeinfo_prop;
+ my $old_minfo = {};
+
+ # Initialize cache on the first call.
+ unless (defined $self->{cached_mergeinfo_rev}) {
+ $self->{cached_mergeinfo_rev} = {};
+ $self->{cached_mergeinfo} = {};
+ }
+
+ my $cached_rev = $self->{cached_mergeinfo_rev}{$old_path};
+ if (defined $cached_rev && $cached_rev == $old_rev) {
+ $old_minfo = $self->{cached_mergeinfo}{$old_path};
+ } else {
+ my $ra = $self->ra;
+ # Give up if $old_path isn't in the repo.
+ # This is probably a merge on a subtree.
+ if ($ra->check_path($old_path, $old_rev) != $SVN::Node::dir) {
+ warn "W: ignoring svn:mergeinfo on $old_path, ",
+ "directory didn't exist in r$old_rev\n";
+ return {};
+ }
+ my (undef, undef, $props) =
+ $self->ra->get_dir($old_path, $old_rev);
+ if (defined $props->{"svn:mergeinfo"}) {
+ my %omi = map {split ":", $_ } split "\n",
+ $props->{"svn:mergeinfo"};
+ $old_minfo = \%omi;
+ }
+ $self->{cached_mergeinfo}{$old_path} = $old_minfo;
+ $self->{cached_mergeinfo_rev}{$old_path} = $old_rev;
+ }
+
+ # Cache the new mergeinfo.
+ $self->{cached_mergeinfo}{$path} = \%minfo;
+ $self->{cached_mergeinfo_rev}{$path} = $rev;
+
+ my %changes = ();
+ foreach my $p (keys %minfo) {
+ my $a = $old_minfo->{$p} || "";
+ my $b = $minfo{$p};
+ # Omit merged branches whose ranges lists are unchanged.
+ next if $a eq $b;
+ # Remove any common range list prefix.
+ ($a ^ $b) =~ /^[\0]*/;
+ my $common_prefix = rindex $b, ",", $+[0] - 1;
+ $changes{$p} = substr $b, $common_prefix + 1;
+ }
+ print STDERR "Checking svn:mergeinfo changes since r$old_rev: ",
+ scalar(keys %minfo), " sources, ",
+ scalar(keys %changes), " changed\n";
+
+ return \%changes;
+}
# note: this function should only be called if the various dirprops
# have actually changed
@@ -1715,14 +1771,15 @@ sub find_extra_svn_parents {
# history. Then, we figure out which git revisions are in
# that tip, but not this
[PATCH 2/2] git-svn: only look at the root path for svn:mergeinfo
Subversion can put mergeinfo on any sub-directory to track cherry-picks.
Since cherry-picks are not represented explicitly in git, git-svn should
just ignore it.
Signed-off-by: Jakob Stoklund Olesen
---
perl/Git/SVN.pm | 29 +
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index d3785ab..0aa4dd3 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -1210,7 +1210,7 @@ sub do_fetch {
unless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {
die "SVN connection failed somewhere...\n";
}
- $self->make_log_entry($rev, \@parents, $ed, $last_rev);
+ $self->make_log_entry($rev, \@parents, $ed, $last_rev, $self->path);
}
sub mkemptydirs {
@@ -1859,21 +1859,18 @@ sub make_log_entry {
my $untracked = $self->get_untracked($ed);
my @parents = @$parents;
- my $ps = $ed->{path_strip} || "";
- for my $path ( grep { m/$ps/ } %{$ed->{dir_prop}} ) {
- my $props = $ed->{dir_prop}{$path};
- if ( $props->{"svk:merge"} ) {
- $self->find_extra_svk_parents
- ($ed, $props->{"svk:merge"}, \@parents);
- }
- if ( $props->{"svn:mergeinfo"} ) {
- my $mi_changes = $self->mergeinfo_changes
- ($parent_path || $path, $parent_rev,
-$path, $rev,
-$props->{"svn:mergeinfo"});
- $self->find_extra_svn_parents
- ($ed, $mi_changes, \@parents);
- }
+ my $props = $ed->{dir_prop}{$self->path};
+ if ( $props->{"svk:merge"} ) {
+ $self->find_extra_svk_parents
+ ($ed, $props->{"svk:merge"}, \@parents);
+ }
+ if ( $props->{"svn:mergeinfo"} ) {
+ my $mi_changes = $self->mergeinfo_changes
+ ($parent_path, $parent_rev,
+$self->path, $rev,
+$props->{"svn:mergeinfo"});
+ $self->find_extra_svn_parents
+ ($ed, $mi_changes, \@parents);
}
open my $un, '>>', "$self->{dir}/unhandled.log" or croak $!;
--
1.8.5.2 (Apple Git-48)
--
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
