[PATCH 3/3] Fix sed usage in tests to work around broken xpg4/sed on Solaris
In 99094a7a, a trivial && breakage was fixed. This exposed a problem with the test when run on Solaris with xpg4/sed that had gone silently undetected since its introduction in e4bd10b2. Solaris' sed executes the requested substitution but prints a warning about the missing newline at the end of the file and exits with status 2. % echo "CHANGE_ME" | \ tr -d "\\012" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/' sed: Missing newline at end of file standard input. change_me % echo $? 2 To work around this, use perl to perform the substitution instead. Signed-off-by: Ben Walton --- t/t9500-gitweb-standalone-no-errors.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh index e94b2f1..a7383fa 100755 --- a/t/t9500-gitweb-standalone-no-errors.sh +++ b/t/t9500-gitweb-standalone-no-errors.sh @@ -290,7 +290,7 @@ test_expect_success 'setup incomplete lines' ' echo "incomplete" | tr -d "\\012" >>file && git commit -a -m "Add incomplete line" && git tag incomplete_lines_add && - sed -e s/CHANGE_ME/change_me/ file+ && + perl -pne "s/CHANGE_ME/change_me/" file >file+ && mv -f file+ file && git commit -a -m "Incomplete context line" && git tag incomplete_lines_ctx && -- 2.1.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/3] Modify tr expressions so that xpg4/tr handles it on Solaris
It seems that xpg4/tr mishandles some strings involving [ not followed
by a character class:
% echo '[::1]' | /usr/xpg4/bin/tr -d '[]'
[::1
% echo '[::1]' | /usr/xpg4/bin/tr -d '['
usr/xpg4/bin/tr: Bad string.
This was breaking two tests. To fix the issue, use the octal
representations of [ and ] instead. Reference the octal expression as
a newly exported variable so that it's shared across tests and more
easily understood when reading it.
Signed-off-by: Ben Walton
---
t/t5500-fetch-pack.sh | 2 +-
t/t5601-clone.sh | 8
t/test-lib.sh | 5 -
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 3a9b775..2db9bde 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,7 +547,7 @@ check_prot_host_port_path () {
*ssh*)
pp=ssh
uah=userandhost
- ehost=$(echo $3 | tr -d "[]")
+ ehost=$(echo $3 | tr -d "$squarebrackets")
diagport="Diag: port=$4"
;;
*)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index bfdaf75..8299d14 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '
#IPv6
for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1]
[user@::1]:
do
- ehost=$(echo $tuah | sed -e "s/1]:/1]/ "| tr -d "[]")
+ ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "$squarebrackets")
test_expect_success "clone ssh://$tuah/home/user/repo" "
test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
"
@@ -454,7 +454,7 @@ done
#IPv6 from home directory
for tuah in ::1 [::1] user@::1 user@[::1] [user@::1]
do
- euah=$(echo $tuah | tr -d "[]")
+ euah=$(echo $tuah | tr -d "$squarebrackets")
test_expect_success "clone ssh://$tuah/~repo" "
test_clone_url ssh://$tuah/~repo $euah '~repo'
"
@@ -463,7 +463,7 @@ done
#IPv6 with port number
for tuah in [::1] user@[::1] [user@::1]
do
- euah=$(echo $tuah | tr -d "[]")
+ euah=$(echo $tuah | tr -d "$squarebrackets")
test_expect_success "clone ssh://$tuah:22/home/user/repo" "
test_clone_url ssh://$tuah:22/home/user/repo '-p 22' $euah
/home/user/repo
"
@@ -472,7 +472,7 @@ done
#IPv6 from home directory with port number
for tuah in [::1] user@[::1] [user@::1]
do
- euah=$(echo $tuah | tr -d "[]")
+ euah=$(echo $tuah | tr -d "$squarebrackets")
test_expect_success "clone ssh://$tuah:22/~repo" "
test_clone_url ssh://$tuah:22/~repo '-p 22' $euah '~repo'
"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 39da9c2..6b5b6cd 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -173,7 +173,10 @@ LF='
# when case-folding filenames
u200c=$(printf '\342\200\214')
-export _x05 _x40 _z40 LF u200c
+# [ and ], for use by tr commands.
+squarebrackets="\133\135"
+
+export _x05 _x40 _z40 LF u200c squarebrackets
# Each test should start with something like this, after copyright notices:
#
--
2.1.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/3] Fix sed usage in tests to work around broken xpg4/sed on Solaris
The space following the last / in a sed command caused Solaris' xpg4/sed to fail, claiming the program was garbled and exit with status 2: % echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ ' sed: command garbled: s/foo/bar/ % echo $? 2 Fix this by simply removing the unnecessary space. Signed-off-by: Ben Walton --- t/t5601-clone.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 8299d14..8b7f8e1 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' ' #IPv6 for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]: do - ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "$squarebrackets") + ehost=$(echo $tuah | sed -e "s/1]:/1]/" | tr -d "$squarebrackets") test_expect_success "clone ssh://$tuah/home/user/repo" " test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo " -- 2.1.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
Adapt some tests to work around broken Solaris tools
This series is a respin of the previous submission, taking feedback into account. -- 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] Modify tr expression so that xpg4/tr handles it on Solaris
It seems that xpg4/tr mishandles some strings involving [ not followed
by a character class:
% echo '[::1]' | /usr/xpg4/bin/tr -d '[]'
[::1
% echo '[::1]' | /usr/xpg4/bin/tr -d '['
usr/xpg4/bin/tr: Bad string.
This was breaking two tests. To fix the issue, use the octal
representations of [ and ] instead.
Signed-off-by: Ben Walton
---
t/t5500-fetch-pack.sh |2 +-
t/t5601-clone.sh |8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 3a9b775..5bc4da9 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,7 +547,7 @@ check_prot_host_port_path () {
*ssh*)
pp=ssh
uah=userandhost
- ehost=$(echo $3 | tr -d "[]")
+ ehost=$(echo $3 | tr -d "\133\135")
diagport="Diag: port=$4"
;;
*)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index bfdaf75..fa6be3c 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '
#IPv6
for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1]
[user@::1]:
do
- ehost=$(echo $tuah | sed -e "s/1]:/1]/ "| tr -d "[]")
+ ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "\133\135")
test_expect_success "clone ssh://$tuah/home/user/repo" "
test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
"
@@ -454,7 +454,7 @@ done
#IPv6 from home directory
for tuah in ::1 [::1] user@::1 user@[::1] [user@::1]
do
- euah=$(echo $tuah | tr -d "[]")
+ euah=$(echo $tuah | tr -d "\133\135")
test_expect_success "clone ssh://$tuah/~repo" "
test_clone_url ssh://$tuah/~repo $euah '~repo'
"
@@ -463,7 +463,7 @@ done
#IPv6 with port number
for tuah in [::1] user@[::1] [user@::1]
do
- euah=$(echo $tuah | tr -d "[]")
+ euah=$(echo $tuah | tr -d "\133\135")
test_expect_success "clone ssh://$tuah:22/home/user/repo" "
test_clone_url ssh://$tuah:22/home/user/repo '-p 22' $euah
/home/user/repo
"
@@ -472,7 +472,7 @@ done
#IPv6 from home directory with port number
for tuah in [::1] user@[::1] [user@::1]
do
- euah=$(echo $tuah | tr -d "[]")
+ euah=$(echo $tuah | tr -d "\133\135")
test_expect_success "clone ssh://$tuah:22/~repo" "
test_clone_url ssh://$tuah:22/~repo '-p 22' $euah '~repo'
"
--
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/2] Fix sed usage in tests to work around broken xpg4/sed on Solaris
The space following the last / in a sed command caused Solaris' xpg4/sed to fail, claiming the program was garbled and exit with status 2: % echo 'foo' | /usr/xpg4/bin/sed -e 's/foo/bar/ ' sed: command garbled: s/foo/bar/ % echo $? 2 Fix this by simply removing the unnecessary space. Additionally, in 99094a7a, a trivial && breakage was fixed. This exposed a problem with the test when run on Solaris with xpg4/sed that had gone silently undetected since its introduction in e4bd10b2. Solaris' sed executes the requested substitution but prints a warning about the missing newline at the end of the file and exits with status 2. % echo "CHANGE_ME" | \ tr -d "\\012" | /usr/xpg4/bin/sed -e 's/CHANGE_ME/change_me/' sed: Missing newline at end of file standard input. change_me % echo $? 2 To work around this, use perl to execute the substitution instead. By using inplace replacement, we can subsequently drop the mv command. Signed-off-by: Ben Walton --- t/t5601-clone.sh |2 +- t/t9500-gitweb-standalone-no-errors.sh |3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index fa6be3c..2583f84 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -445,7 +445,7 @@ test_expect_success 'clone ssh://host.xz:22/~repo' ' #IPv6 for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]: do - ehost=$(echo $tuah | sed -e "s/1]:/1]/ " | tr -d "\133\135") + ehost=$(echo $tuah | sed -e "s/1]:/1]/" | tr -d "\133\135") test_expect_success "clone ssh://$tuah/home/user/repo" " test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo " diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh index e94b2f1..eb264f9 100755 --- a/t/t9500-gitweb-standalone-no-errors.sh +++ b/t/t9500-gitweb-standalone-no-errors.sh @@ -290,8 +290,7 @@ test_expect_success 'setup incomplete lines' ' echo "incomplete" | tr -d "\\012" >>file && git commit -a -m "Add incomplete line" && git tag incomplete_lines_add && - sed -e s/CHANGE_ME/change_me/ file+ && - mv -f file+ file && + perl -pi -e "s/CHANGE_ME/change_me/" file && git commit -a -m "Incomplete context line" && git tag incomplete_lines_ctx && echo "Dominus regit me," >file && -- 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] Use unsigned char to squash compiler warnings
Sun Studio on Solaris issues warnings about improper initialization
values being used when defining tolower_trans_tbl in
ctype.c. tolower_trans_tbl is defined as char[], which studio's
compiler defaults to signed char[] due to the Solaris ABI. To resolve
this, instead of supplying -xchar or another option at build time,
declare tolower_trans_tbl as unsigned char. Update all appropriate
references to the new type.
Signed-off-by: Ben Walton
---
ctype.c | 2 +-
git-compat-util.h | 2 +-
kwset.c | 8
kwset.h | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/ctype.c b/ctype.c
index 0bfebb4..fc0225c 100644
--- a/ctype.c
+++ b/ctype.c
@@ -30,7 +30,7 @@ const unsigned char sane_ctype[256] = {
};
/* For case-insensitive kwset */
-const char tolower_trans_tbl[256] = {
+const unsigned char tolower_trans_tbl[256] = {
0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
diff --git a/git-compat-util.h b/git-compat-util.h
index 3455c5e..5eae2b2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -694,7 +694,7 @@ static inline size_t xsize_t(off_t len)
}
/* in ctype.c, for kwset users */
-extern const char tolower_trans_tbl[256];
+extern const unsigned char tolower_trans_tbl[256];
/* Sane ctype - no locale, and works with signed chars */
#undef isascii
diff --git a/kwset.c b/kwset.c
index a0f49b3..e6236a0 100644
--- a/kwset.c
+++ b/kwset.c
@@ -80,13 +80,13 @@ struct kwset
struct trie *next[NCHAR];/* Table of children of the root. */
char *target;/* Target string if there's only one. */
int mind2; /* Used in Boyer-Moore search for one string. */
- char const *trans; /* Character translation table. */
+ unsigned char const *trans; /* Character translation table. */
};
/* Allocate and initialize a keyword set object, returning an opaque
pointer to it. Return NULL if memory is not available. */
kwset_t
-kwsalloc (char const *trans)
+kwsalloc (unsigned char const *trans)
{
struct kwset *kwset;
@@ -381,7 +381,7 @@ kwsprep (kwset_t kws)
register struct kwset *kwset;
register int i;
register struct trie *curr;
- register char const *trans;
+ register unsigned char const *trans;
unsigned char delta[NCHAR];
kwset = (struct kwset *) kws;
@@ -590,7 +590,7 @@ cwexec (kwset_t kws, char const *text, size_t len, struct
kwsmatch *kwsmatch)
register int d;
register char const *end, *qlim;
register struct tree const *tree;
- register char const *trans;
+ register unsigned char const *trans;
accept = NULL;
diff --git a/kwset.h b/kwset.h
index a21b2ea..61a134f 100644
--- a/kwset.h
+++ b/kwset.h
@@ -39,7 +39,7 @@ typedef struct kwset_t* kwset_t;
if enough memory cannot be obtained. The argument if non-NULL
specifies a table of character translations to be applied to all
pattern and search text. */
-extern kwset_t kwsalloc(char const *);
+extern kwset_t kwsalloc(unsigned char const *);
/* Incrementally extend the keyword set to include the given string.
Return NULL for success, or an error message. Remember an index
--
1.9.1
--
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] Use wc instead of awk to count subtrees in t0090-cache-tree
The awk statements previously used in this test weren't compatible
with the native versions of awk on Solaris:
echo "dir" | /bin/awk -v c=0 '$1 {++c} END {print c}'
awk: syntax error near line 1
awk: bailing out near line 1
echo "dir" | /usr/xpg4/bin/awk -v c=0 '$1 {++c} END {print c}'
0
And with GNU awk for comparison:
echo "dir" | /opt/csw/gnu/awk -v c=0 '$1 {++c} END {print c}'
1
Instead of modifying the awk code to work, use wc -w instead as that
is both adequate and simpler.
Signed-off-by: Ben Walton
---
t/t0090-cache-tree.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 067f4c6..f2b1c9c 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -22,7 +22,7 @@ generate_expected_cache_tree_rec () {
# ls-files might have foo/bar, foo/bar/baz, and foo/bar/quux
# We want to count only foo because it's the only direct child
subtrees=$(git ls-files|grep /|cut -d / -f 1|uniq) &&
- subtree_count=$(echo "$subtrees"|awk -v c=0 '$1 {++c} END {print c}') &&
+ subtree_count=$(echo "$subtrees"|wc -w) &&
entries=$(git ls-files|wc -l) &&
printf "SHA $dir (%d entries, %d subtrees)\n" "$entries"
"$subtree_count" &&
for subtree in $subtrees
--
1.9.1
--
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] Ensure SHELL_PATH is the hash bang for test suite askpass helper script.
The askpass script that is created for use by the test suite should
use SHELL_PATH for its hash bang instead of /bin/sh. Commit 5a4352024
introduced the use of idioms not supported in some legacy /bin/sh
implementations.
Use write_script to ensure this happens automatically. This lets us
remove the chmod step as well, since write_script handles that.
Signed-off-by: Ben Walton
---
t/lib-credential.sh | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 9e7d796..d8e41f7 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -278,12 +278,10 @@ helper_test_timeout() {
'
}
-cat >askpass <<\EOF
-#!/bin/sh
+write_script askpass <<\EOF
echo >&2 askpass: $*
what=$(echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z)
echo "askpass-$what"
EOF
-chmod +x askpass
GIT_ASKPASS="$PWD/askpass"
export GIT_ASKPASS
--
1.9.1
--
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] Use SHELL_PATH as hash bang in test suite askpass helper script.
The askpass script that is created for use by the test suite should
use SHELL_PATH for its hash bang instead of /bin/sh. Commit 5a4352024
introduced the use of idioms not supported in some legacy /bin/sh
implementations.
Signed-off-by: Ben Walton
---
t/lib-credential.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index 9e7d796..ca4a6de 100755
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -278,8 +278,8 @@ helper_test_timeout() {
'
}
-cat >askpass <<\EOF
-#!/bin/sh
+echo "#!$SHELL_PATH" >askpass
+cat >>askpass <<\EOF
echo >&2 askpass: $*
what=$(echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z)
echo "askpass-$what"
--
1.9.1
--
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] Do not make trace.c/getnanotime an inlined function
Oracle Studio compilers don't allow for static variables in functions
that are defined to be inline. GNU C does permit this. Let's reference
the C99 standard though, which doesn't allow for inline functions to
contain modifiable static variables.
Signed-off-by: Ben Walton
---
trace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/trace.c b/trace.c
index b6f25a2..4778608 100644
--- a/trace.c
+++ b/trace.c
@@ -385,7 +385,7 @@ static inline uint64_t gettimeofday_nanos(void)
* Returns nanoseconds since the epoch (01/01/1970), for performance tracing
* (i.e. favoring high precision over wall clock time accuracy).
*/
-inline uint64_t getnanotime(void)
+uint64_t getnanotime(void)
{
static uint64_t offset;
if (offset > 1) {
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Fix a bug in compat/bswap.h endianness detection
The changes to make detection of endianness more portable had a bug that breaks on (at least) Solaris x86. The bug appears to be a simple copy/paste typo. It checks for _BIG_ENDIAN and not _LITTLE_ENDIAN for both the case where we would decide the system is big endian and little endian. Instead, the second test should be for _LITTLE_ENDIAN and not _BIG_ENDIAN. Two fixes were possible: 1. Change the negation order of the conditions in the second test. 2. Reverse the order of the conditions in the second test. Use the second option so that the condition we expect is always a positive check. Signed-off-by: Ben Walton --- I think that this should be applied to cb/byte-swap and re-merged with next. compat/bswap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/bswap.h b/compat/bswap.h index c4293db..f6fd9a6 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -120,7 +120,7 @@ static inline uint64_t git_bswap64(uint64_t x) # if defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN) # define GIT_BYTE_ORDER GIT_BIG_ENDIAN -# elif defined(_BIG_ENDIAN) && !defined(_LITTLE_ENDIAN) +# elif defined(_LITTLE_ENDIAN) && !defined(_BIG_ENDIAN) # define GIT_BYTE_ORDER GIT_LITTLE_ENDIAN # else # error "Cannot determine endianness" -- 1.9.1 -- 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] Change sed i\ usage to something Solaris' sed can handle
Solaris' sed was choking on the i\ commands used in t4015-diff-whitespace as it couldn't parse the program properly. Modify two uses of sed that worked in GNU sed but not Solaris' (/usr/bin or /usr/xpg4/bin) to an equivalent form that is handled properly by both. Signed-off-by: Ben Walton --- This addresses Andreas' comment about the extraneous \. Sorry, I misunderstood the original comment. t/t4015-diff-whitespace.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 3fb4b97..604a838 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -145,7 +145,7 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect test_expect_success 'ignore-blank-lines: only new lines' ' test_seq 5 >x && git update-index x && - test_seq 5 | sed "/3/i \\ + test_seq 5 | sed "/3/i\\ " >x && git diff --ignore-blank-lines >out && >expect && @@ -155,7 +155,8 @@ test_expect_success 'ignore-blank-lines: only new lines' ' test_expect_success 'ignore-blank-lines: only new lines with space' ' test_seq 5 >x && git update-index x && - test_seq 5 | sed "/3/i \ " >x && + test_seq 5 | sed "/3/i\\ + " >x && git diff -w --ignore-blank-lines >out && >expect && test_cmp out expect -- 1.8.3.2 -- 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] Avoid difference in tr semantics between System V and BSD
Ignore this version. The immediate followup quotes PERL_PATH.
On Mon, Oct 28, 2013 at 9:40 PM, Ben Walton wrote:
> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
> semantics for tr whereby string1's length is truncated to the length
> of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
> see string2 padded to the length of string1 using the final character
> in string2. POSIX explicitly doesn't specify the correct behavior
> here, making both equally valid.
>
> This difference means that Solaris' native tr implementations produce
> different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
> tests in t0008-ignores.sh.
>
> Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
>
> Instead, use perl to perform these transliterations which means we
> don't need to worry about the difference at all. Since we're replacing
> tr with perl, we also use perl to replace the sed invocations used to
> transform the files.
>
> Replace four identical transforms with a function named
> broken_c_unquote. Replace the other two identical transforms with a
> fuction named broken_c_unquote_verbose.
>
> Signed-off-by: Ben Walton
> ---
> t/t0008-ignores.sh | 30 ++
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index 181513a..45f9396 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -37,6 +37,14 @@ test_stderr () {
> test_cmp "$HOME/expected-stderr" "$HOME/stderr"
> }
>
> +broken_c_unquote () {
> + $PERL_PATH -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
> +}
> +
> +broken_c_unquote_verbose () {
> + $PERL_PATH -pe 's/ "/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
> +}
> +
> stderr_contains () {
> regexp="$1"
> if grep "$regexp" "$HOME/stderr"
> @@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
> $global_excludes:2:!globaltwo b/globaltwo
> EOF
>
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
> - tr "\n" "\0" >stdin0
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
> - tr "\n" "\0" >expected-default0
> -sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
> - tr ":\t\n" "\0" >expected-verbose0
> +broken_c_unquote stdin >stdin0
> +
> +broken_c_unquote expected-default >expected-default0
> +
> +broken_c_unquote_verbose expected-verbose >expected-verbose0
>
> test_expect_success '--stdin' '
> expect_from_stdin @@ -692,12 +699,11 @@ EOF
> grep -v '^:: ' expected-all >expected-verbose
> sed -e 's/.* //' expected-verbose >expected-default
>
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
> - tr "\n" "\0" >stdin0
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
> - tr "\n" "\0" >expected-default0
> -sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
> - tr ":\t\n" "\0" >expected-verbose0
> +broken_c_unquote stdin >stdin0
> +
> +broken_c_unquote expected-default >expected-default0
> +
> +broken_c_unquote_verbose expected-verbose >expected-verbose0
>
> test_expect_success '--stdin from subdirectory' '
> expect_from_stdin --
> 1.8.1.2
>
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
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] t/README: tests can use perl even with NO_PERL
On Mon, Oct 28, 2013 at 9:04 PM, Jonathan Nieder wrote:
> Jeff King wrote:
>
>> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
>> everywhere, and not simply do:
>>
>> perl () {
>> "$PERL_PATH" "$@"
>> }
>>
>> in test-lib.sh?
>
> Sounds like a nice potential improvement to me. :)
+1.
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
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] Avoid difference in tr semantics between System V and BSD
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.
This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.
Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.
Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.
Signed-off-by: Ben Walton
---
...Forgot to quote PERL_PATH in the previous iteration.
t/t0008-ignores.sh | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..b4d98e6 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
test_cmp "$HOME/expected-stderr" "$HOME/stderr"
}
+broken_c_unquote () {
+ "$PERL_PATH" -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+ "$PERL_PATH" -pe 's/"/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
stderr_contains () {
regexp="$1"
if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
$global_excludes:2:!globaltwo b/globaltwo
EOF
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin' '
expect_from_stdin expected-verbose
sed -e 's/.* //' expected-verbose >expected-default
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin from subdirectory' '
expect_from_stdin http://vger.kernel.org/majordomo-info.html
[PATCH] Avoid difference in tr semantics between System V and BSD
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.
This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.
Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.
Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.
Signed-off-by: Ben Walton
---
t/t0008-ignores.sh | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..45f9396 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
test_cmp "$HOME/expected-stderr" "$HOME/stderr"
}
+broken_c_unquote () {
+ $PERL_PATH -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+ $PERL_PATH -pe 's/ "/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
stderr_contains () {
regexp="$1"
if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
$global_excludes:2:!globaltwo b/globaltwo
EOF
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin' '
expect_from_stdin expected-verbose
sed -e 's/.* //' expected-verbose >expected-default
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin from subdirectory' '
expect_from_stdin http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
On Mon, Oct 28, 2013 at 9:04 PM, Ben Walton wrote:
> I'm happy to defer to your judgement on this - If you'd like the tests
> wrapped, I'll do so.
>
> Thanks
> -Ben
>
> On Mon, Oct 28, 2013 at 7:08 PM, Junio C Hamano wrote:
>> Jonathan Nieder writes:
>>
>>> Johannes Sixt wrote:
>>>
>>>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>>>> that perl is not available. Shouldn't we do that here, too?
>>>
>>> I think the tests assume there's a perl present even when the PERL
>>> prereq isn't present already. E.g.:
>>>
>>> nul_to_q () {
>>> "$PERL_PATH" -pe 'y/\000/Q/'
>>> }
>>>
>>> So in practice the PERL prereq just means "NO_PERL wasn't set", or
>>> in other words, "commands that use perl work". Maybe something
>>> like the following would help?
>>>
>>> Signed-off-by: Jonathan Nieder
>>>
>>> diff --git i/t/README w/t/README
>>> index 2167125..54cd064 100644
>>> --- i/t/README
>>> +++ w/t/README
>>> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in
>>> the "Test harness
>>> library" section above and the "test_have_prereq" function for how to
>>> use these, and "test_set_prereq" for how to define your own.
>>>
>>> - - PERL & PYTHON
>>> + - PYTHON
>>>
>>> - Git wasn't compiled with NO_PERL=YesPlease or
>>> - NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
>>> - these.
>>> + Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
>>> + need Python with this.
>>> +
>>> + - PERL
>>> +
>>> + Git wasn't compiled with NO_PERL=YesPlease.
>>> +
>>> + Even without the PERL prerequisite, tests can assume there is a
>>> + usable perl interpreter at $PERL_PATH, though it need not be
>>> + particularly modern.
>>> +
>>> + Wrap tests for commands implemented in Perl with this.
>>
>> Sounds like a sensible thing to address, but I first parsed it as
>>
>> Wrap (tests for (commands implemented in Perl)) with this.
>>
>> while I think the readers are expected to parse it as
>>
>>Wrap ((tests for commands) implemented in Perl) with this.
>>
Per the other discussion about replacing all PERL_PATH with a shell
function named perl, should I update this patch to use $PERL_PATH in
the meantime so that it can be batch updated when the function is
added in a separate patch?
Thanks
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
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] Change sed i\ usage to something Solaris' sed can handle
On Mon, Oct 28, 2013 at 5:39 PM, Andreas Schwab wrote: > Ben Walton writes: > >> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh >> index 3fb4b97..0126154 100755 >> --- a/t/t4015-diff-whitespace.sh >> +++ b/t/t4015-diff-whitespace.sh >> @@ -145,7 +145,8 @@ test_expect_success 'another test, with >> --ignore-space-at-eol' 'test_cmp expect >> test_expect_success 'ignore-blank-lines: only new lines' ' >> test_seq 5 >x && >> git update-index x && >> - test_seq 5 | sed "/3/i \\ >> + test_seq 5 | sed "/3/i\\ >> +\ >> " >x && > > Why do you need the \? Since it is inside double quotes the shell > will remove it during expansion. It's an escape. Without it, sed throws: sed: -e expression #1, char 5: expected \ after `a', `c' or `i' Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] Avoid difference in tr semantics between System V and BSD
I'm happy to defer to your judgement on this - If you'd like the tests
wrapped, I'll do so.
Thanks
-Ben
On Mon, Oct 28, 2013 at 7:08 PM, Junio C Hamano wrote:
> Jonathan Nieder writes:
>
>> Johannes Sixt wrote:
>>
>>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>>> that perl is not available. Shouldn't we do that here, too?
>>
>> I think the tests assume there's a perl present even when the PERL
>> prereq isn't present already. E.g.:
>>
>> nul_to_q () {
>> "$PERL_PATH" -pe 'y/\000/Q/'
>> }
>>
>> So in practice the PERL prereq just means "NO_PERL wasn't set", or
>> in other words, "commands that use perl work". Maybe something
>> like the following would help?
>>
>> Signed-off-by: Jonathan Nieder
>>
>> diff --git i/t/README w/t/README
>> index 2167125..54cd064 100644
>> --- i/t/README
>> +++ w/t/README
>> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the
>> "Test harness
>> library" section above and the "test_have_prereq" function for how to
>> use these, and "test_set_prereq" for how to define your own.
>>
>> - - PERL & PYTHON
>> + - PYTHON
>>
>> - Git wasn't compiled with NO_PERL=YesPlease or
>> - NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
>> - these.
>> + Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
>> + need Python with this.
>> +
>> + - PERL
>> +
>> + Git wasn't compiled with NO_PERL=YesPlease.
>> +
>> + Even without the PERL prerequisite, tests can assume there is a
>> + usable perl interpreter at $PERL_PATH, though it need not be
>> + particularly modern.
>> +
>> + Wrap tests for commands implemented in Perl with this.
>
> Sounds like a sensible thing to address, but I first parsed it as
>
> Wrap (tests for (commands implemented in Perl)) with this.
>
> while I think the readers are expected to parse it as
>
>Wrap ((tests for commands) implemented in Perl) with this.
>
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
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] Avoid difference in tr semantics between System V and BSD
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.
This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.
Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.
Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.
Signed-off-by: Ben Walton
---
t/t0008-ignores.sh | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..4ebda99 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
test_cmp "$HOME/expected-stderr" "$HOME/stderr"
}
+broken_c_unquote () {
+ perl -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+ perl -pe 's/"/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
stderr_contains () {
regexp="$1"
if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
$global_excludes:2:!globaltwo b/globaltwo
EOF
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin' '
expect_from_stdin expected-verbose
sed -e 's/.* //' expected-verbose >expected-default
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin from subdirectory' '
expect_from_stdin http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid broken Solaris tr
On Tue, Jun 18, 2013 at 11:31 PM, Junio C Hamano wrote:
Sorry for the very slow reply. This got lost in my inbox and I forgot about it.
> Ben Walton writes:
>
>> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) fail to handle the case
>> where the first argument is a multi-character set and the second is a
>> single null character.
>
> Almost all the tr invocations look like converting LF to NUL, except
> for two that squash a colon ':', HT and LF all to NUL. Is Solaris's
> tr fine with the former but not the latter?
In retrospect, this isn't brokenness, just a difference in System V vs
BSD semantics for tr, both of which are allowed by POSIX since the
behaviour in question is specifically unspecified by the standard. The
System V behaviour is to require a 1:1 map between string1 and string2
transformations whereas BSD behaviour (when len(string2) <
len(string1)) is to pad string2 with the last character in string2
until the lengths are equal.
>
>> We make this change globally in t0008-ignores instead of just for the
>> cases where it matters in order to maintain consistency.
>
> I am not suggesting to keep 'tr "\n" "\0"', but just wanted to make
> sure I am reading the first paragraph correctly. If we are
> rewriting, we should do so consistently.
>
>> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0
>
> What is -pne? Is it the same as -pe?
>
> tr/\n/\0/ (or y/\n/\0/) may be more faithful to the original.
>
>
>> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \
>> +expected-default0
>
> Ditto. We may want to give the same script used in the above two
> (and twice again in the later hunk) more descriptive name, e.g.
>
> broken_c_unquote () {
> perl -pe '... that script ...' "$@"
> }
>
> broken_c_quote stdin >stdin0
>
> Side note: the script is broken as a generic C-unquote function in
> multiple ways. It does not work if it has more than one backslash
> quoted characters, it does not understand \t, \b, \015, \\, etc. to
> name two.
>
> But the breakage does not matter for the strings used in the test
> vector.
I've updated the patch and will forward it shortly.
Thanks
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
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] Change sed i\ usage to something Solaris' sed can handle
Solaris' sed was choking on the i\ commands used in t4015-diff-whitespace as it couldn't parse the program properly. Modify two uses of sed that worked in GNU sed but not Solaris' (/usr/bin or /usr/xpg4/bin) to an equivalent form that is handled properly by both. Signed-off-by: Ben Walton --- t/t4015-diff-whitespace.sh | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh index 3fb4b97..0126154 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -145,7 +145,8 @@ test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect test_expect_success 'ignore-blank-lines: only new lines' ' test_seq 5 >x && git update-index x && - test_seq 5 | sed "/3/i \\ + test_seq 5 | sed "/3/i\\ +\ " >x && git diff --ignore-blank-lines >out && >expect && @@ -155,7 +156,8 @@ test_expect_success 'ignore-blank-lines: only new lines' ' test_expect_success 'ignore-blank-lines: only new lines with space' ' test_seq 5 >x && git update-index x && - test_seq 5 | sed "/3/i \ " >x && + test_seq 5 | sed "/3/i\\ + " >x && git diff -w --ignore-blank-lines >out && >expect && test_cmp out expect -- 1.8.1.2 -- 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] Avoid broken Solaris tr
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) fail to handle the case where the first argument is a multi-character set and the second is a single null character. Use perl to perform these substitutions instead. Now that we're using perl for the transliteration, we might as well replace the sed invocations with it too. We make this change globally in t0008-ignores instead of just for the cases where it matters in order to maintain consistency. Signed-off-by: Ben Walton --- t/t0008-ignores.sh | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh index a56db80..9e4987e 100755 --- a/t/t0008-ignores.sh +++ b/t/t0008-ignores.sh @@ -552,12 +552,13 @@ cat <<-EOF >expected-verbose $global_excludes:2:!globaltwo b/globaltwo EOF -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \ - tr "\n" "\0" >stdin0 -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \ - tr "\n" "\0" >expected-default0 -sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \ - tr ":\t\n" "\0" >expected-verbose0 +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0 + +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \ +expected-default0 + +perl -pne 's/ "/ /; s/\\//; s/"$//; s/[:\t\n]/\0/g' expected-verbose > \ +expected-verbose0 test_expect_success '--stdin' ' expect_from_stdin expected-verbose sed -e 's/.* //' expected-verbose >expected-default -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \ - tr "\n" "\0" >stdin0 -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \ - tr "\n" "\0" >expected-default0 -sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \ - tr ":\t\n" "\0" >expected-verbose0 +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0 + +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \ +expected-default0 + +perl -pne 's/ "/ /; s/\\//; s/"$//; s/[:\t\n]/\0/g' expected-verbose > \ +expected-verbose0 test_expect_success '--stdin from subdirectory' ' expect_from_stdin http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Fix a portability issue with git-cvsimport
This is my (long overdue) re-roll of the series that fixes a portability issue with git-cvsimport's use of strftime. It also fixes a but in the original implementation of get_tz (now get_tz_offset). I ended up taking taking only part of the implementation suggested by Junio. The only usage of get_tz_offset is by git-cvsimport and Git::SVN::Log currently. There are tests that validate it works currently so I didn't add anything additional. If the git-cvsimport tests are removed, there are no tests remaining that exercise the code full as the SVN tests use UTC times. Ben Walton (3): Move Git::SVN::get_tz to Git::get_tz_offset Fix get_tz_offset to properly handle DST boundary cases Avoid non-portable strftime format specifiers in git-cvsimport git-cvsimport.perl |5 - perl/Git.pm | 23 +++ perl/Git/SVN.pm | 12 ++-- perl/Git/SVN/Log.pm |8 ++-- 4 files changed, 35 insertions(+), 13 deletions(-) -- 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/3] Avoid non-portable strftime format specifiers in git-cvsimport
Neither %s or %z are portable strftime format specifiers. There is no
need for %s in git-cvsimport as the supplied time is already in
seconds since the epoch. For %z, use the function get_tz_offset
provided by Git.pm instead.
Signed-off-by: Ben Walton
---
git-cvsimport.perl |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 0a31ebd..344f120 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -26,6 +26,7 @@ use IO::Socket;
use IO::Pipe;
use POSIX qw(strftime tzset dup2 ENOENT);
use IPC::Open2;
+use Git qw(get_tz_offset);
$SIG{'PIPE'}="IGNORE";
set_timezone('UTC');
@@ -864,7 +865,9 @@ sub commit {
}
set_timezone($author_tz);
- my $commit_date = strftime("%s %z", localtime($date));
+ # $date is in the seconds since epoch format
+ my $tz_offset = get_tz_offset($date);
+ my $commit_date = "$date $tz_offset";
set_timezone('UTC');
$ENV{GIT_AUTHOR_NAME} = $author_name;
$ENV{GIT_AUTHOR_EMAIL} = $author_email;
--
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/3] Move Git::SVN::get_tz to Git::get_tz_offset
This function has utility outside of the SVN module for any routine
that needs the equivalent of GNU strftime's %z formatting option.
Move it to the top-level Git.pm so that non-SVN modules don't need to
import the SVN module to use it.
The rename makes the purpose of the function clearer.
Signed-off-by: Ben Walton
---
perl/Git.pm | 23 +++
perl/Git/SVN.pm | 12 ++--
perl/Git/SVN/Log.pm |8 ++--
3 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..5649bcc 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,6 +59,7 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
+get_tz_offset
temp_acquire temp_release temp_reset temp_path);
@@ -102,6 +103,7 @@ use Error qw(:try);
use Cwd qw(abs_path cwd);
use IPC::Open2 qw(open2);
use Fcntl qw(SEEK_SET SEEK_CUR);
+use Time::Local qw(timelocal);
}
@@ -511,6 +513,27 @@ C). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+
+=item get_tz_offset ( TIME )
+
+Return the time zone offset from GMT in the form +/-HHMM where HH is
+the number of hours from GMT and MM is the number of minutes. This is
+the equivalent of what strftime("%z", ...) would provide on a GNU
+platform.
+
+If TIME is not supplied, the current local time is used.
+
+=cut
+
+sub get_tz_offset {
+ # some systmes don't handle or mishandle %z, so be creative.
+ my $t = shift || time;
+ my $gm = timelocal(gmtime($t));
+ my $sign = qw( + + - )[ $t <=> $gm ];
+ return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
+}
+
+
=item prompt ( PROMPT , ISPASSWORD )
Query user C and return answer from user.
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 490e330..0ebc68a 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -11,7 +11,6 @@ use Carp qw/croak/;
use File::Path qw/mkpath/;
use File::Copy qw/copy/;
use IPC::Open3;
-use Time::Local;
use Memoize; # core since 5.8.0, Jul 2002
use Memoize::Storable;
use POSIX qw(:signal_h);
@@ -22,6 +21,7 @@ use Git qw(
command_noisy
command_output_pipe
command_close_pipe
+get_tz_offset
);
use Git::SVN::Utils qw(
fatal
@@ -1311,14 +1311,6 @@ sub get_untracked {
\@out;
}
-sub get_tz {
- # some systmes don't handle or mishandle %z, so be creative.
- my $t = shift || time;
- my $gm = timelocal(gmtime($t));
- my $sign = qw( + + - )[ $t <=> $gm ];
- return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
-}
-
# parse_svn_date(DATE)
#
# Given a date (in UTC) from Subversion, return a string in the format
@@ -1351,7 +1343,7 @@ sub parse_svn_date {
delete $ENV{TZ};
}
- my $our_TZ = get_tz();
+ my $our_TZ = get_tz_offset();
# This converts $epoch_in_UTC into our local timezone.
my ($sec, $min, $hour, $mday, $mon, $year,
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 3cc1c6f..3f8350a 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -2,7 +2,11 @@ package Git::SVN::Log;
use strict;
use warnings;
use Git::SVN::Utils qw(fatal);
-use Git qw(command command_oneline command_output_pipe command_close_pipe);
+use Git qw(command
+ command_oneline
+ command_output_pipe
+ command_close_pipe
+ get_tz_offset);
use POSIX qw/strftime/;
use constant commit_log_separator => ('-' x 72) . "\n";
use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
@@ -119,7 +123,7 @@ sub run_pager {
sub format_svn_date {
my $t = shift || time;
require Git::SVN;
- my $gmoff = Git::SVN::get_tz($t);
+ my $gmoff = get_tz_offset($t);
return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)",
localtime($t));
}
--
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/3] Fix get_tz_offset to properly handle DST boundary cases
When passed a local time that was on the boundary of a DST change,
get_tz_offset returned a GMT offset that was incorrect (off by one
hour). This is because the time was converted to GMT and then back to
a time stamp via timelocal() which cannot disambiguate boundary cases
as noted in its documentation.
Modify this algorithm, using an approach suggested by Junio C Hamano
that obtains the GMT time stamp by using timegm(localtime()) instead
of timelocal(gmtime()). This avoids the ambigious conversion and
allows a correct time to be returned on every occassion.
Signed-off-by: Ben Walton
---
perl/Git.pm |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 5649bcc..a56d1e7 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -103,7 +103,7 @@ use Error qw(:try);
use Cwd qw(abs_path cwd);
use IPC::Open2 qw(open2);
use Fcntl qw(SEEK_SET SEEK_CUR);
-use Time::Local qw(timelocal);
+use Time::Local qw(timegm);
}
@@ -528,8 +528,8 @@ If TIME is not supplied, the current local time is used.
sub get_tz_offset {
# some systmes don't handle or mishandle %z, so be creative.
my $t = shift || time;
- my $gm = timelocal(gmtime($t));
- my $sign = qw( + + - )[ $t <=> $gm ];
+ my $gm = timegm(localtime($t));
+ my $sign = qw( + + - )[ $gm <=> $t ];
return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
}
--
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 2/3] Allow Git::get_tz_offset to properly handle DST boundary times
On Thu, Jan 17, 2013 at 7:09 PM, Junio C Hamano wrote:
> Ben Walton writes:
>
>> The Git::get_tz_offset is meant to provide a workalike replacement for
>> the GNU strftime %z format specifier. The algorithm used failed to
>> properly handle DST boundary cases.
>>
>> For example, the unix time 1162105199 in CST6CDT saw set_tz_offset
>> improperly return -0600 instead of -0500.
>>
>> TZ=CST6CDT date -d @1162105199 +"%c %z"
>> Sun 29 Oct 2006 01:59:59 AM CDT -0500
>>
>> $ zdump -v /usr/share/zoneinfo/CST6CDT | grep 2006
>> /usr/share/zoneinfo/CST6CDT Sun Apr 2 07:59:59 2006 UTC = Sun Apr 2
>> 01:59:59 2006 CST isdst=0 gmtoff=-21600
>> /usr/share/zoneinfo/CST6CDT Sun Apr 2 08:00:00 2006 UTC = Sun Apr 2
>> 03:00:00 2006 CDT isdst=1 gmtoff=-18000
>> /usr/share/zoneinfo/CST6CDT Sun Oct 29 06:59:59 2006 UTC = Sun Oct 29
>> 01:59:59 2006 CDT isdst=1 gmtoff=-18000
>> /usr/share/zoneinfo/CST6CDT Sun Oct 29 07:00:00 2006 UTC = Sun Oct 29
>> 01:00:00 2006 CST isdst=0 gmtoff=-21600
>>
>> To determine how many hours/minutes away from GMT a particular time
>> was, we calculated the gmtime() of the requested time value and then
>> used Time::Local's timelocal() function to turn the GMT-based time
>> back into a scalar value representing seconds from the epoch. Because
>> GMT has no daylight savings time, timelocal() cannot handle the
>> ambiguous times that occur at DST boundaries since there are two
>> possible correct results.
>>
>> To work around the ambiguity at these boundaries, we must take into
>> account the pre and post conversion values for is_dst as provided by
>> both the original time value and the value that has been run through
>> timelocal(). If the is_dst field of the two times disagree then we
>> must modify the value returned from timelocal() by an hour in the
>> correct direction.
>
> It seems to me that it is a very roundabout way. It may be correct,
> but it is unclear why the magic +/-3600 shift is the right solution
> and I suspect even you wouldn't notice if I sent you back your patch
> with a slight change to swap $gm += 3600 and $gm -= 3600 lines ;-).
>
> For that timestamp in question, the human-readable representation of
> gmtime($t) and localtime($t) look like these two strings:
>
> my $t = 1162105199;
> print gmtime($t), "\n";# Sun Oct 29 06:59:59 2006
> print localtime($t), "\n"; # Sun Oct 29 01:59:59 2006
>
> As a human, you can easily see that these two stringified timestamps
> look 5 hours apart. Think how you managed to do so.
>
> If we convert these back to the seconds-since-epoch, as if these
> broken-down times were both in a single timezone that does not have
> any DST issues, you can get the offset (in seconds) by subtraction,
> and that is essentially the same as the way in which your eyes saw
> they are 5 hours apart, no? In other words, why do you need to run
> timelocal() at all?
>
> my $t = 1162105199;
> my $lct = timegm(localtime($t));
> # of course, timegm(gmtime($t)) == $t
>
> my $minutes = int(($lct - $t)/60);
> my $sign "+";
> if ($minutes < 0) {
> $sign = "-";
> $minutes = -$minutes;
> }
> my $hours = int($minutes/60);
> $minutes -= $hours * 60;
> sprintf("%s%02d%02d", $sign, $hours, $minutes);
>
> Confused...
>
>>
>> Signed-off-by: Ben Walton
>> ---
>> perl/Git.pm | 20
>> 1 file changed, 20 insertions(+)
>>
>> diff --git a/perl/Git.pm b/perl/Git.pm
>> index 5649bcc..788b9b4 100644
>> --- a/perl/Git.pm
>> +++ b/perl/Git.pm
>> @@ -528,7 +528,27 @@ If TIME is not supplied, the current local time is used.
>> sub get_tz_offset {
>> # some systmes don't handle or mishandle %z, so be creative.
>> my $t = shift || time;
>> + # timelocal() has a problem when it comes to DST ambiguity so
>> + # times that are on a DST boundary cannot be properly converted
>> + # using it. we will possibly adjust its result depending on whehter
>> + # pre and post conversions agree on DST
>> my $gm = timelocal(gmtime($t));
>> +
>> + # we need to know whether we were originally in DST or not
>> + my $orig_dst = (localtime($t))[8];
>> + # and also whether timelocal thinks we're in DST
>> + my $conv_dst = (localtime($gm))[8];
>> +
>> + # re-adjust $gm based on the DST value for the two times we're
>&g
Re: [PATCH 1/3] Move Git::SVN::get_tz to Git::get_tz_offset
On Wed, Jan 16, 2013 at 3:37 PM, Junio C Hamano wrote:
> Ben Walton writes:
>
>> +sub get_tz_offset {
>> + # some systmes don't handle or mishandle %z, so be creative.
>
> Hmph. I wonder if we can use %z if it is handled correctly and fall
> back to this code only on platforms that are broken?
That would be perfectly acceptable to me. The reason I set it up to
always run through this function here is that when I originally added
this function for git-svn, I'd made it conditional and Eric Wong
preferred that the function be used exclusively[1]. I opted to take
the same approach here to keep things congrous.
If it were to be conditional, I think I'd add a variable to the build
system and have the code leverage that at runtime instead of the
try/except approach I attempted in 2009.
Thanks
-Ben
[1]
http://lists-archives.com/git/683572-git-svn-fix-for-systems-without-strftime-z.html
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] Avoid non-portable strftime format specifiers in git-cvsimport
On Wed, Jan 16, 2013 at 1:53 AM, Chris Rorvick wrote: > On Tue, Jan 15, 2013 at 5:10 PM, Ben Walton wrote: >> Neither %s or %z are portable strftime format specifiers. There is no >> need for %s in git-cvsimport as the supplied time is already in >> seconds since the epoch. For %z, use the function get_tz_offset >> provided by Git.pm instead. > > Out of curiosity, which platforms are affected? Assuming DST is a 1 > hour shift (patch 2/3) is not necessarily portable either, though this > currently appears to only affect a small island off of the coast of > Australia. :-) My primary motivation on this change was for solaris. %s isn't supported in 10 (not sure about 11) and %z was only added in 10. The issue affects other older platforms as well. Good point about the 1 hour assumption. Is it worth hacking in additional logic to handle Lord Howe Island? I think it's likely a case of "in for a penny, in for a pound" but that could lead to madness when it comes to time zones. Either way, the function behaves better now than before. (I wasn't aware of the half hour oddball wrt to DST, so I learned something new here too!) Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] Fix a portability issue with git-cvsimport
This patch series started as a quick fix for the use of %s and %z in git-cvsimport but grew slightly when I realized that the get_tz (get_tz_offset after this series) function used by Git::SVN didn't properly handle DST boundary conditions. I realize that Eric Raymond is working to deprecate the current iteration of git-cvsimport so this series may be only partially worthwhile. (If the cvsps 2 vs 3 issue does require a fallback git-cvsimport script then maybe the whole series is still valid?) I'm not attached to the current git-cvsimport so if the third patch isn't useful then maybe the only the second patch is worthwhile (modified to correct the function in its current location). Currently, the DST boundary functionality is exercised by the git-cvsimport tests. If those go away as part of Eric's work then another test that monitors this condition should be added. I can do that as part of this series if it seems the right way to go. Ben Walton (3): Move Git::SVN::get_tz to Git::get_tz_offset Allow Git::get_tz_offset to properly handle DST boundary times Avoid non-portable strftime format specifiers in git-cvsimport git-cvsimport.perl |5 - perl/Git.pm | 43 +++ perl/Git/SVN.pm | 12 ++-- perl/Git/SVN/Log.pm |8 ++-- 4 files changed, 55 insertions(+), 13 deletions(-) -- 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/3] Allow Git::get_tz_offset to properly handle DST boundary times
The Git::get_tz_offset is meant to provide a workalike replacement for
the GNU strftime %z format specifier. The algorithm used failed to
properly handle DST boundary cases.
For example, the unix time 1162105199 in CST6CDT saw set_tz_offset
improperly return -0600 instead of -0500.
TZ=CST6CDT date -d @1162105199 +"%c %z"
Sun 29 Oct 2006 01:59:59 AM CDT -0500
$ zdump -v /usr/share/zoneinfo/CST6CDT | grep 2006
/usr/share/zoneinfo/CST6CDT Sun Apr 2 07:59:59 2006 UTC = Sun Apr 2
01:59:59 2006 CST isdst=0 gmtoff=-21600
/usr/share/zoneinfo/CST6CDT Sun Apr 2 08:00:00 2006 UTC = Sun Apr 2
03:00:00 2006 CDT isdst=1 gmtoff=-18000
/usr/share/zoneinfo/CST6CDT Sun Oct 29 06:59:59 2006 UTC = Sun Oct 29
01:59:59 2006 CDT isdst=1 gmtoff=-18000
/usr/share/zoneinfo/CST6CDT Sun Oct 29 07:00:00 2006 UTC = Sun Oct 29
01:00:00 2006 CST isdst=0 gmtoff=-21600
To determine how many hours/minutes away from GMT a particular time
was, we calculated the gmtime() of the requested time value and then
used Time::Local's timelocal() function to turn the GMT-based time
back into a scalar value representing seconds from the epoch. Because
GMT has no daylight savings time, timelocal() cannot handle the
ambiguous times that occur at DST boundaries since there are two
possible correct results.
To work around the ambiguity at these boundaries, we must take into
account the pre and post conversion values for is_dst as provided by
both the original time value and the value that has been run through
timelocal(). If the is_dst field of the two times disagree then we
must modify the value returned from timelocal() by an hour in the
correct direction.
Signed-off-by: Ben Walton
---
perl/Git.pm | 20
1 file changed, 20 insertions(+)
diff --git a/perl/Git.pm b/perl/Git.pm
index 5649bcc..788b9b4 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -528,7 +528,27 @@ If TIME is not supplied, the current local time is used.
sub get_tz_offset {
# some systmes don't handle or mishandle %z, so be creative.
my $t = shift || time;
+ # timelocal() has a problem when it comes to DST ambiguity so
+ # times that are on a DST boundary cannot be properly converted
+ # using it. we will possibly adjust its result depending on whehter
+ # pre and post conversions agree on DST
my $gm = timelocal(gmtime($t));
+
+ # we need to know whether we were originally in DST or not
+ my $orig_dst = (localtime($t))[8];
+ # and also whether timelocal thinks we're in DST
+ my $conv_dst = (localtime($gm))[8];
+
+ # re-adjust $gm based on the DST value for the two times we're
+ # handling.
+ if ($orig_dst != $conv_dst) {
+ if ($orig_dst == 1) {
+ $gm -= 3600;
+ } else {
+ $gm += 3600;
+ }
+ }
+
my $sign = qw( + + - )[ $t <=> $gm ];
return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
}
--
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/3] Avoid non-portable strftime format specifiers in git-cvsimport
Neither %s or %z are portable strftime format specifiers. There is no
need for %s in git-cvsimport as the supplied time is already in
seconds since the epoch. For %z, use the function get_tz_offset
provided by Git.pm instead.
Signed-off-by: Ben Walton
---
git-cvsimport.perl |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 0a31ebd..344f120 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -26,6 +26,7 @@ use IO::Socket;
use IO::Pipe;
use POSIX qw(strftime tzset dup2 ENOENT);
use IPC::Open2;
+use Git qw(get_tz_offset);
$SIG{'PIPE'}="IGNORE";
set_timezone('UTC');
@@ -864,7 +865,9 @@ sub commit {
}
set_timezone($author_tz);
- my $commit_date = strftime("%s %z", localtime($date));
+ # $date is in the seconds since epoch format
+ my $tz_offset = get_tz_offset($date);
+ my $commit_date = "$date $tz_offset";
set_timezone('UTC');
$ENV{GIT_AUTHOR_NAME} = $author_name;
$ENV{GIT_AUTHOR_EMAIL} = $author_email;
--
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/3] Move Git::SVN::get_tz to Git::get_tz_offset
This function has utility outside of the SVN module for any routine
that needs the equivalent of GNU strftime's %z formatting option.
Move it to the top-level Git.pm so that non-SVN modules don't need to
import the SVN module to use it.
The rename makes the purpose of the function clearer.
Signed-off-by: Ben Walton
---
perl/Git.pm | 23 +++
perl/Git/SVN.pm | 12 ++--
perl/Git/SVN/Log.pm |8 ++--
3 files changed, 31 insertions(+), 12 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 931047c..5649bcc 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,6 +59,7 @@ require Exporter;
command_bidi_pipe command_close_bidi_pipe
version exec_path html_path hash_object git_cmd_try
remote_refs prompt
+get_tz_offset
temp_acquire temp_release temp_reset temp_path);
@@ -102,6 +103,7 @@ use Error qw(:try);
use Cwd qw(abs_path cwd);
use IPC::Open2 qw(open2);
use Fcntl qw(SEEK_SET SEEK_CUR);
+use Time::Local qw(timelocal);
}
@@ -511,6 +513,27 @@ C). Useful mostly only internally.
sub html_path { command_oneline('--html-path') }
+
+=item get_tz_offset ( TIME )
+
+Return the time zone offset from GMT in the form +/-HHMM where HH is
+the number of hours from GMT and MM is the number of minutes. This is
+the equivalent of what strftime("%z", ...) would provide on a GNU
+platform.
+
+If TIME is not supplied, the current local time is used.
+
+=cut
+
+sub get_tz_offset {
+ # some systmes don't handle or mishandle %z, so be creative.
+ my $t = shift || time;
+ my $gm = timelocal(gmtime($t));
+ my $sign = qw( + + - )[ $t <=> $gm ];
+ return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
+}
+
+
=item prompt ( PROMPT , ISPASSWORD )
Query user C and return answer from user.
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 59215fa..8c84560 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -11,7 +11,6 @@ use Carp qw/croak/;
use File::Path qw/mkpath/;
use File::Copy qw/copy/;
use IPC::Open3;
-use Time::Local;
use Memoize; # core since 5.8.0, Jul 2002
use Memoize::Storable;
use POSIX qw(:signal_h);
@@ -22,6 +21,7 @@ use Git qw(
command_noisy
command_output_pipe
command_close_pipe
+get_tz_offset
);
use Git::SVN::Utils qw(
fatal
@@ -1311,14 +1311,6 @@ sub get_untracked {
\@out;
}
-sub get_tz {
- # some systmes don't handle or mishandle %z, so be creative.
- my $t = shift || time;
- my $gm = timelocal(gmtime($t));
- my $sign = qw( + + - )[ $t <=> $gm ];
- return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
-}
-
# parse_svn_date(DATE)
#
# Given a date (in UTC) from Subversion, return a string in the format
@@ -1351,7 +1343,7 @@ sub parse_svn_date {
delete $ENV{TZ};
}
- my $our_TZ = get_tz();
+ my $our_TZ = get_tz_offset();
# This converts $epoch_in_UTC into our local timezone.
my ($sec, $min, $hour, $mday, $mon, $year,
diff --git a/perl/Git/SVN/Log.pm b/perl/Git/SVN/Log.pm
index 3cc1c6f..3f8350a 100644
--- a/perl/Git/SVN/Log.pm
+++ b/perl/Git/SVN/Log.pm
@@ -2,7 +2,11 @@ package Git::SVN::Log;
use strict;
use warnings;
use Git::SVN::Utils qw(fatal);
-use Git qw(command command_oneline command_output_pipe command_close_pipe);
+use Git qw(command
+ command_oneline
+ command_output_pipe
+ command_close_pipe
+ get_tz_offset);
use POSIX qw/strftime/;
use constant commit_log_separator => ('-' x 72) . "\n";
use vars qw/$TZ $limit $color $pager $non_recursive $verbose $oneline
@@ -119,7 +123,7 @@ sub run_pager {
sub format_svn_date {
my $t = shift || time;
require Git::SVN;
- my $gmoff = Git::SVN::get_tz($t);
+ my $gmoff = get_tz_offset($t);
return strftime("%Y-%m-%d %H:%M:%S $gmoff (%a, %d %b %Y)",
localtime($t));
}
--
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] Avoid using non-POSIX cp options
t/3600-rm was using the -a option to cp. This option is a GNU extention and is not portable. Instead, use just -R (no -p necessary). Signed-off-by: Ben Walton --- t/t3600-rm.sh |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 06f6384..37bf5f1 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -474,7 +474,7 @@ test_expect_success 'rm of a conflicted populated submodule with a .git director git submodule update && (cd submod && rm .git && - cp -a ../.git/modules/sub .git && + cp -R ../.git/modules/sub .git && GIT_WORK_TREE=. git config --unset core.worktree ) && test_must_fail git merge conflict2 && @@ -508,7 +508,7 @@ test_expect_success 'rm of a populated submodule with a .git directory fails eve git submodule update && (cd submod && rm .git && - cp -a ../.git/modules/sub .git && + cp -R ../.git/modules/sub .git && GIT_WORK_TREE=. git config --unset core.worktree ) && test_must_fail git rm submod && @@ -606,7 +606,7 @@ test_expect_success 'rm of a populated nested submodule with a nested .git direc git submodule update --recursive && (cd submod/subsubmod && rm .git && - cp -a ../../.git/modules/sub/modules/sub .git && + cp -R ../../.git/modules/sub/modules/sub .git && GIT_WORK_TREE=. git config --unset core.worktree ) && test_must_fail git rm submod && -- 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] Fix t9200 on case insensitive file systems
On Fri, Oct 26, 2012 at 5:18 PM, Torsten Bögershausen wrote: > t/t9200-git-cvsexportcommit.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh > index b59be9a..69934b2 100755 > --- a/t/t9200-git-cvsexportcommit.sh > +++ b/t/t9200-git-cvsexportcommit.sh > @@ -19,7 +19,7 @@ then > test_done > fi > > -CVSROOT=$PWD/cvsroot > +CVSROOT=$PWD/tmpcvsroot FWIW, this looks obviously correct given the code snippet from the cvs version you shared the other day. Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git config error message
On Sat, Oct 27, 2012 at 9:32 AM, Angelo Borsotti wrote: Hi Angelo, > I wrote "value", but I meant "name". The first example I made contains > a name with a nonexistent section, the second a name with a > nonexistent key. This still wouldn't be an error condition though, especially in terms of how "git config" should treat it. It should be up to the consumer of the information to display, or not, any error or diagnostics that don't result from either a bad request (your first case) or a malformed configuration file. This fits with the callback nature of how the config file is parsed by builtin tools. The exit code from "git config" with a missing key is enough for the consumer to make this decision. This is just my take on it, but I think the current approach makes sense. Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] Use character class for sed expression instead of \s
Hi Torsten, On Thu, Oct 25, 2012 at 5:28 PM, Torsten Bögershausen wrote: > BTW: While we are talking CVS: (I installed a fresh version) > cvs --version > Concurrent Versions System (CVS) 1.11.23 (client/server) I have 1.12.13-MirDebian-8 here. > And t9200 fails: > git checkout t9200-git-cvsexportcommit.sh > tb@birne:~/projects/git/git.pu/t> ./t9200-git-cvsexportcommit.sh > cvs [init aborted]: Cannot initialize repository under existing CVSROOT: > `/Users/tb/projects/git/git.pu/t/trash directory.t9200-git-cvsexportcommit' > FATAL: Unexpected exit with code 1 I'm not able to reproduce this manually...are you able to make it fail this way outside of the test harness? $ CVSROOT=$PWD/bw $ export CVSROOT $ mkdir $CVSROOT && cvs init && echo ok ok $ rm -rf $CVSROOT $ cvs init && echo ok ok >> (cvs init || mkdir "$CVSROOT" && cvs init ) && If your version of cvs fails the checks above in manual testing, we could see if there is a flag that works in all (old and new) versions to override the failure if CVSROOT exists. Otherwise, this isn't a bad fix, I don't think. If your version does fail the manual checks, I think it's likely a regression that was introduced and later reverted. I don't see those strings inside my cvs binary at all...? HTH. Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] t/t5400-send-pack: Use POSIX options to cp for portability
On Mon, Oct 8, 2012 at 6:45 PM, Junio C Hamano wrote: > Junio C Hamano writes: > >>> Thanks, but is "-p" essential for this test to pass, or can we get >>> away with just "-R"? >> >> Besides, when you spot a potential problem, please ask "git grep" >> to catch them all. > > In other words, how about doing this instead? This works. I was responding to a failing test so I didn't look for other instances. Clearly I should have...I must not be exercising those other tests. Acked-By: Ben Walton Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] t/t5400-send-pack: Use POSIX options to cp for portability
Avoid a GNU-ism in the cp options used by t5400-send-pack. Change -a to -pR. Signed-off-by: Ben Walton --- t/t5400-send-pack.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh index 250c720..65b3b0f 100755 --- a/t/t5400-send-pack.sh +++ b/t/t5400-send-pack.sh @@ -159,7 +159,7 @@ test_expect_success 'receive-pack runs auto-gc in remote repo' ' git commit -a -m "Second commit" && git repack ) && - cp -a parent child && + cp -pR parent child && ( # Set the child to auto-pack if more than one pack exists cd child && -- 1.7.12 -- 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] Remove the hard coded length limit on variable names in config files
Previously while reading the variable names in config files, there was
a 256 character limit with at most 128 of those characters being used
by the section header portion of the variable name. This limitation
was only enforced while reading the config files. It was possible to
write a config file that was not subsequently readable.
Instead of enforcing this limitation for both reading and writing,
remove it entirely by changing the var member of the config_file
struct to a strbuf instead of a fixed length buffer. Update all of
the parsing functions in config.c to use the strbuf instead of the
static buffer.
The parsing functions that returned the base length of the variable
name now return simply 0 for success and -1 for failure. The base
length information is obtained through the strbuf's len member.
We now send the buf member of the strbuf to external callback
functions to preserve the external api. None of the external callers
rely on the old size limitation for sizing their own buffers so
removing the limit should have no externally visible effect.
Signed-off-by: Ben Walton
---
config.c | 59 +--
1 file changed, 29 insertions(+), 30 deletions(-)
diff --git a/config.c b/config.c
index 08e47e2..24fb2d2 100644
--- a/config.c
+++ b/config.c
@@ -10,8 +10,6 @@
#include "strbuf.h"
#include "quote.h"
-#define MAXNAME (256)
-
typedef struct config_file {
struct config_file *prev;
FILE *f;
@@ -19,7 +17,7 @@ typedef struct config_file {
int linenr;
int eof;
struct strbuf value;
- char var[MAXNAME];
+ struct strbuf var;
} config_file;
static config_file *cf;
@@ -260,7 +258,7 @@ static inline int iskeychar(int c)
return isalnum(c) || c == '-';
}
-static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
+static int get_value(config_fn_t fn, void *data, struct strbuf *name)
{
int c;
char *value;
@@ -272,11 +270,9 @@ static int get_value(config_fn_t fn, void *data, char
*name, unsigned int len)
break;
if (!iskeychar(c))
break;
- name[len++] = tolower(c);
- if (len >= MAXNAME)
- return -1;
+ strbuf_addch(name, tolower(c));
}
- name[len] = 0;
+
while (c == ' ' || c == '\t')
c = get_next_char();
@@ -288,10 +284,10 @@ static int get_value(config_fn_t fn, void *data, char
*name, unsigned int len)
if (!value)
return -1;
}
- return fn(name, value, data);
+ return fn(name->buf, value, data);
}
-static int get_extended_base_var(char *name, int baselen, int c)
+static int get_extended_base_var(struct strbuf *name, int c)
{
do {
if (c == '\n')
@@ -302,7 +298,7 @@ static int get_extended_base_var(char *name, int baselen,
int c)
/* We require the format to be '[base "extension"]' */
if (c != '"')
return -1;
- name[baselen++] = '.';
+ strbuf_addch(name, '.');
for (;;) {
int c = get_next_char();
@@ -315,37 +311,31 @@ static int get_extended_base_var(char *name, int baselen,
int c)
if (c == '\n')
goto error_incomplete_line;
}
- name[baselen++] = c;
- if (baselen > MAXNAME / 2)
- return -1;
+ strbuf_addch(name, c);
}
/* Final ']' */
if (get_next_char() != ']')
return -1;
- return baselen;
+ return 0;
error_incomplete_line:
cf->linenr--;
return -1;
}
-static int get_base_var(char *name)
+static int get_base_var(struct strbuf *name)
{
- int baselen = 0;
-
for (;;) {
int c = get_next_char();
if (cf->eof)
return -1;
if (c == ']')
- return baselen;
+ return 0;
if (isspace(c))
- return get_extended_base_var(name, baselen, c);
+ return get_extended_base_var(name, c);
if (!iskeychar(c) && c != '.')
return -1;
- if (baselen > MAXNAME / 2)
- return -1;
- name[baselen++] = tolower(c);
+ strbuf_addch(name, tolower(c));
}
}
@@ -353,7 +343,7 @@ static int git_parse_file(config_fn_t fn, void *data)
{
int comment = 0;
int baselen = 0;
- char *var = cf->var;
+ struct strbuf *var = &cf->v
Re: [PATCH] Remove the hard coded length limit on variable names in config files
Hi Michael, > The patch doesn't apply to the current master; it appears to have been > built against master 883a2a3504 (2012-02-23) or older. It will have to > be rebased to the current master. Junio had asked that it be based on maint so that's what I (thought I?) did. I'm happy to redo it against master if that's better though. > The preferred format for multiline comments in the git project is > > /* > * Truncate the var name back to the section header prior to > * grabbing the suffix part of the name and the value. > */ Oops; Will fix. > In the old code, get_base_var() read the string into var and returned > var's length (or -1 on error). The fact that the length of var was > first "reset" to zero is somewhat implicit in the fact that no length > parameter is being passed to get_base_var(). > > But in the new version, get_base_var() is passed a strbuf. Often, > operations with strbufs append to the strbuf, and this is what I first > assumed. It took me a while to realize that get_base_var() calls > strbuf_reset() before getting to work. Moreover, get_base_var() still > returns the length of what it found, which is redundant with a strbuf > and therefore unexpected. So when the return value of get_base_var() is > stored into baselen, it is not really obvious that it is the string's > length. Ok, that's a fair criticism. When I was creating the patch, I thought that placing the strbuf_reset in get_base_var() seemed nicer as it matched the baselen = 0 which effectively reset the old character array. Your point is well taken though and I think it makes sense to switch things around the way you've suggested. > Finally, I realize that the MAXNAME constant was not exported and I > can't find the old length limits documented anywhere, but I nevertheless > worry a little bit that one of the users of the config API has a > built-in assumption that names can never be longer than 256 characters > (for example, a config_fn_t function might try to store the name into a > fixed-length buffer). Hopefully such code would never have been written > or accepted, but...? If you have thought about this or audited the > callers, please mention that in your commit message. I did look through the code to see if anything was relying on fixed size buffers and I didn't see anything. I'll update the commit message with this info too. I'll send a modified patch shortly. Thanks for the review! -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] Remove the hard coded length limit on variable names in config files
Previously while reading the variable names in config files, there was
a 256 character limit with at most 128 of those characters being used
by the section header portion of the variable name. This limitation
was only enforced while reading the config files. It was possible to
write a config file that was not subsequently readable.
Instead of enforcing this limitation for both reading and writing,
remove it entirely by changing the var member of the config_file
struct to a strbuf instead of a fixed length buffer. Update all of
the parsing functions in config.c to use the strbuf instead of the
static buffer. Send the buf member of the strbuf to external callback
functions to preserve the external api.
Signed-off-by: Ben Walton
---
Hi Junio,
(Sorry that this patch took so long to submit. I've been busy moving.)
I think this should remove the length limitations enforced while reading
configuration file variable names.
Thanks
-Ben
config.c | 50 +++---
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/config.c b/config.c
index 40f9c6d..ee860a7 100644
--- a/config.c
+++ b/config.c
@@ -10,8 +10,6 @@
#include "strbuf.h"
#include "quote.h"
-#define MAXNAME (256)
-
typedef struct config_file {
struct config_file *prev;
FILE *f;
@@ -19,7 +17,7 @@ typedef struct config_file {
int linenr;
int eof;
struct strbuf value;
- char var[MAXNAME];
+ struct strbuf var;
} config_file;
static config_file *cf;
@@ -191,7 +189,7 @@ static inline int iskeychar(int c)
return isalnum(c) || c == '-';
}
-static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
+static int get_value(config_fn_t fn, void *data, struct strbuf *name)
{
int c;
char *value;
@@ -203,11 +201,9 @@ static int get_value(config_fn_t fn, void *data, char
*name, unsigned int len)
break;
if (!iskeychar(c))
break;
- name[len++] = tolower(c);
- if (len >= MAXNAME)
- return -1;
+ strbuf_addch(name, tolower(c));
}
- name[len] = 0;
+
while (c == ' ' || c == '\t')
c = get_next_char();
@@ -219,10 +215,10 @@ static int get_value(config_fn_t fn, void *data, char
*name, unsigned int len)
if (!value)
return -1;
}
- return fn(name, value, data);
+ return fn(name->buf, value, data);
}
-static int get_extended_base_var(char *name, int baselen, int c)
+static int get_extended_base_var(struct strbuf *name, int c)
{
do {
if (c == '\n')
@@ -233,7 +229,7 @@ static int get_extended_base_var(char *name, int baselen,
int c)
/* We require the format to be '[base "extension"]' */
if (c != '"')
return -1;
- name[baselen++] = '.';
+ strbuf_addch(name, '.');
for (;;) {
int c = get_next_char();
@@ -246,34 +242,30 @@ static int get_extended_base_var(char *name, int baselen,
int c)
if (c == '\n')
return -1;
}
- name[baselen++] = c;
- if (baselen > MAXNAME / 2)
- return -1;
+ strbuf_addch(name, c);
}
/* Final ']' */
if (get_next_char() != ']')
return -1;
- return baselen;
+ return name->len;
}
-static int get_base_var(char *name)
+static int get_base_var(struct strbuf *name)
{
- int baselen = 0;
+ strbuf_reset(name);
for (;;) {
int c = get_next_char();
if (cf->eof)
return -1;
if (c == ']')
- return baselen;
+ return name->len;
if (isspace(c))
- return get_extended_base_var(name, baselen, c);
+ return get_extended_base_var(name, c);
if (!iskeychar(c) && c != '.')
return -1;
- if (baselen > MAXNAME / 2)
- return -1;
- name[baselen++] = tolower(c);
+ strbuf_addch(name, tolower(c));
}
}
@@ -281,7 +273,7 @@ static int git_parse_file(config_fn_t fn, void *data)
{
int comment = 0;
int baselen = 0;
- char *var = cf->var;
+ struct strbuf *var = &cf->var;
/* U+FEFF Byte Order Mark in UTF8 */
static const unsigned char *utf8_bom = (unsigned char *) "\xef\xbb\xbf";
@@ -320,14 +312,16 @@ static int git_parse_file(config_fn_t fn, void *data)
Re: Unable to clone GIT project
> If I had to guess, I'd say it was ssh, the library is installed in a > non-standard place (e.g., because he built them as a regular user and > put them in his home directory), and LD_LIBRARY_PATH does not get set > properly by ssh for the incoming ssh session. This would be my guess as well. If LD_LIBRARY_PATH is being relied upon though, a more stable solution would be to rebuild git with "-Wl,-rpath /path/containing/libiconv" in the LD_OPTIONS environment. That would remove the need for LD_LIBRARY_PATH at runtime. Any other libraries linked in non-standard locations should also have a similar option if the path to the library differs. HTH. Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- 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] Prevent git-config from storing section keys that are too long
Excerpts from Junio C Hamano's message of Thu Sep 06 21:33:20 -0400 2012: Hi Junio, > > identifiers generated from keys like: > > > > url./some/really/long/path.insteadOf > > > > could overrun the current limit. It's not a common case, of course, > > or this issue would have been found sooner. Would doubling the > > current limit be out of the question? > > Is there a reason to have _any_ limitation? It is not like we store > configuration data by allocating one file per item (in which case we > may be limited by the filesystem limit for direntry size), so if it > is not too much trouble, I think the right thing to do is to lift > the limitation altogether, possibly using strbuf instead of a > statically sized array of characters. I thought it made sense to impose some sort of bound here but removing the limit wouldn't encourage the use of ridiculously long names so lifting it entirely shouldn't hurt. Any chosen limit would always be somewhat arbitrary. I had considered extending it to (PATHMAX + x) where x would also be arbitrary as that would allow any valid url./path/max.insteadOf type setting but that didn't seem like a good approach. Removing the limit is a much better choice... > Of course, once you write a very long entry using a newer version of > Git, the resulting configuration file may end up unusable by older > version of Git, so a patch to implement such a change may need to be > based on older maintenance release (say maint-1.7.9) and then merged > upwards, but otherwise I do not offhand see a compatibility downside > of such a change. I'm ok with this approach and will put an altered patch together shortly. I think it's fairly unlikely, but not impossible, that anyone creating a config file with long key names would be in a situation where someone else couldn't read that same config file. I'll still base the change on maint-1.7.9 as suggested though. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Prevent git-config from storing section keys that are too long
Key names have a length limit defined by MAXNAME in config.c. When
reading the config file, we reserve half of this limit for the section
identifier and the other half for the key name within that section.
For example, if setting a key named url.foo.insteadOf, url.foo may use
at most half of MANXNAME.
The parser will throw an error if this condition is violated.
This patch ensures that git-config enforces the same restriction
during the creation of a section identifier so that it doesn't allow
the generate a configuration file that cannot be re-read later.
This patch also adds a test to t1303-wacky-config to catch any future
issues with this check.
Signed-off-by: Ben Walton
---
Hi All,
I happened to notice this while running the test suite in a deeply
nested directory...
The check for baselen exceeding half of MAXNAME could be done earlier
in the function but doing it late allowed the error message to be
clearer without extra hassle.
I also wonder if MAXNAME should be increased somewhat. Section
identifiers generated from keys like:
url./some/really/long/path.insteadOf
could overrun the current limit. It's not a common case, of course,
or this issue would have been found sooner. Would doubling the
current limit be out of the question?
Thanks
-Ben
config.c|8
t/t1303-wacky-config.sh |4
2 files changed, 12 insertions(+)
diff --git a/config.c b/config.c
index 2b706ea..d3f4854 100644
--- a/config.c
+++ b/config.c
@@ -1276,6 +1276,14 @@ int git_config_parse_key(const char *key, char
**store_key, int *baselen_)
}
(*store_key)[i] = 0;
+ if (baselen > MAXNAME / 2) {
+ /* ok to destroy this value now since it will be freed */
+ (*store_key)[baselen] = '\0';
+ error("section identifier for key is too long (> %d): %s",
+ MAXNAME / 2, *store_key);
+ goto out_free_ret_1;
+ }
+
return 0;
out_free_ret_1:
diff --git a/t/t1303-wacky-config.sh b/t/t1303-wacky-config.sh
index 46103a1..12f0850 100755
--- a/t/t1303-wacky-config.sh
+++ b/t/t1303-wacky-config.sh
@@ -47,4 +47,8 @@ test_expect_success 'do not crash on special long config
line' '
check section.key "$LONG_VALUE"
'
+test_expect_success 'do not accept long section identifiers for key names' '
+ test_must_fail git config
some.REALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlongREALLYlong.key
value
+'
+
test_done
--
1.7.9.5
--
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] Enable HAVE_DEV_TTY for Solaris
Excerpts from Jeff King's message of Tue Aug 07 00:10:26 -0400 2012: > Signed-off-by: Jeff King Acked-by: Ben Walton I agree with your assesment that this is the right way to go (vs migrating out of stdio) for now. If the changes Tay needs to make require the migration then it can become part of that series. Otherwise those changes would just be change for change's sake at this time. If my HAVE_DEV_TTY enabling patch for Solaris is added on top of this, I'm a happy camper. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Enable HAVE_DEV_TTY for Solaris
Now that git_terminal_prompt can cleanly interact with /dev/tty on Solaris, enable HAVE_DEV_TTY so that this code path is used for credential reading instead of relying on the crippled getpass(). Signed-off-by: Ben Walton --- This is a follow up to Jeff's patch that fixes git_terminal_prompt on Solaris. I don't have 5.6 or 5.7 for testing but I believe this should be valid for both of those releases as well. Makefile |1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 15d1319..6b0c961 100644 --- a/Makefile +++ b/Makefile @@ -1014,6 +1014,7 @@ ifeq ($(uname_S),SunOS) NO_REGEX = YesPlease NO_FNMATCH_CASEFOLD = YesPlease NO_MSGFMT_EXTENDED_OPTIONS = YesPlease + HAVE_DEV_TTY = YesPlease ifeq ($(uname_R),5.6) SOCKLEN_T = int NO_HSTRERROR = YesPlease -- 1.7.10.3 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid crippled getpass function on Solaris
Excerpts from Jeff King's message of Mon Aug 06 20:35:41 -0400 2012:
> ---
> compat/terminal.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/compat/terminal.c b/compat/terminal.c
> index 6d16c8f..bbb038d 100644
> --- a/compat/terminal.c
> +++ b/compat/terminal.c
> @@ -59,6 +59,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
>
> r = strbuf_getline(&buf, fh, '\n');
> if (!echo) {
> +fseek(fh, SEEK_CUR, 0);
> putc('\n', fh);
> fflush(fh);
> }
Acked-by: Ben Walton
That looks good to me. I'm able to clone a password protected https
repository and the prompting works as I'd expect.
Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302
--
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] Avoid crippled getpass function on Solaris
Excerpts from Jeff King's message of Mon Aug 06 18:42:22 -0400 2012: > +if (strbuf_getwholeline(sb, fd, term)) Shouldn't this be strbuf_getwholeline_fd though? Otherwise, I think this is a good addition. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Avoid crippled getpass function on Solaris
Excerpts from Jeff King's message of Mon Aug 06 18:31:13 -0400 2012: > Looking over the code, I recall now why I used stdio: strbuf_getline > requires it. These days we have strbuf_getwholeline_fd. It's > slightly less efficient (it has to read() a character at a time), > but since we are talking about a few characters of keyboard input, > it should be OK. I noticed the same requirement. I'm currently building/testing a patch that switches from FILE -> fd and also to strbuf_getwholeline_fd. Personally, I like this approach more than calling an open function multiple times. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Avoid crippled getpass function on Solaris
Excerpts from Jeff King's message of Mon Aug 06 17:34:04 -0400 2012: > On Mon, Aug 06, 2012 at 05:31:00PM -0400, Ben Walton wrote: > > > > I'm happy to spend a few cycles on it. I don't have access to any > > > real Solaris boxes these days, but I imagine I can get OpenSolaris > > > running under VirtualBox without too much trouble... > > > > I'm also happy to do this work and have ready access to Solaris 8-11. > > I'm currently in pkgutil hell trying to remember how to get a working > gcc on Solaris. Bleh. What kind of OS doesn't ship with a compiler by > default? :) One that's losing mindshare and isn't gaining traction? *g* Feel free to message me offline if you need a hand with that. > > Would it be reasonable to support using getpassphrase() if > > !HAVE_DEV_TTY in addition to making the HAVE_DEV_TTY code function > > on Solaris too? That would provide a harm reduction for Solaris > > users that (for some reason) disabled the nicer interface...but > > maybe it's too much? > > If you think there is a reason that somebody on Solaris might not > want to use HAVE_DEV_TTY, then I guess it makes sense. But I can't > really think of one, so I'd be just as happy to leave it until > somebody does. No, I can't think of a _good_ reason to do that either. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Avoid crippled getpass function on Solaris
Excerpts from Jeff King's message of Mon Aug 06 15:39:58 -0400 2012: > On Sun, Aug 05, 2012 at 10:35:06PM -0400, Ben Walton wrote: > > > Excerpts from Junio C Hamano's message of Sun Aug 05 21:59:48 -0400 2012: > > > Wouldn't > > > > > > #if solaris > > > #define getpass getpassphrase > > > #endif > > > > > > without anything else be more than sufficient? > > > > Yes, it would, but I was hoping to make it more explicit that the > > function getpass may be substituted with something else. > > I don't think that's important. Either the thing is a drop-in replica of > getpass, or it is not. In the former case, it's OK for it to be > transparent that it has been replaced. In the latter case, it should not > be a #define replacement at all, but should be its own alternative in > compat/terminal.c (just like HAVE_DEV_TTY is). From my reading of > getpassphrase, it does seem to be a drop-in replacement. It is a drop in replacement (identical signature) so that's no issue. I can simplify this to the form suggested by Junio if that's preferrable. My idea of explicitness was for the programmer though. I think it's easier for someone else reading this code later to realize that the GETPASS function may change depending on conditions. But, as I said, I'm not adverse to changing this in any way, I simply want the improved functionality and am not married to the approach. (I considered about three different ways of doing this before submitting.) > So I'm OK conceptually with the patch if we can't do any better. But > getpass still sucks. It doesn't handle echoing, and it may or may > not fall back to reading from stdin if the tty isn't available > (which is disastrous for remote-curl, whose stdin is speaking the > remote-helper protocol to git). So I'd really prefer to make > HAVE_DEV_TTY work with Solaris if we can. I poked through getpass in the opensolaris code and it did fall back to reading from stdin[1] if there were issues opening /dev/tty so there is room for trouble here. That doesn't mean that other versions of Solaris did it this way, but it's fairly likely that they did given the culture. > I'm happy to spend a few cycles on it. I don't have access to any > real Solaris boxes these days, but I imagine I can get OpenSolaris > running under VirtualBox without too much trouble... I'm also happy to do this work and have ready access to Solaris 8-11. Would it be reasonable to support using getpassphrase() if !HAVE_DEV_TTY in addition to making the HAVE_DEV_TTY code function on Solaris too? That would provide a harm reduction for Solaris users that (for some reason) disabled the nicer interface...but maybe it's too much? > PS If we do go the getpassphrase route, does it make sense to >introduce HAVE_GETPASSPHRASE? We usually try to provide one layer >of indirection by naming our #defines after features, and then >connecting systems to the feature defines via the Makefile. But >maybe Solaris is the only system that has getpassphrase. I had considered this but thought that it was too heavy weight given that I'm not aware of other platforms that have this naming split depending on desired buffer size and that getpass on most platforms won't have this crippling size limitation. If it's worth improving both paths for the HAVE_DEV_TTY code on Solaris, I can add this support if that's considered the better approach. Thanks -Ben [1] http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libbc/libc/gen/common/getpass.c#65 -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Avoid crippled getpass function on Solaris
Excerpts from Tay Ray Chuan's message of Sun Aug 05 21:56:55 -0400 2012:
> > I've also briefly dabbled with getting Solaris to simply use the
> > HAVE_DEV_TTY code path but the terminal echo stuff hasn't worked
> > nicely for me just yet. (It reads the password with nothing
> > echoed but then displays the string after reading the newline.)
> > This might still be a better approach in the future, but for now,
> > having long password reading capability will still be a benefit to
> > users on this platform.
>
> Replacing
>
> if (!echo) {
> putc('\n', fh);
> fflush(fh);
> }
>
> with
>
> if (!echo)
> write(term_fd, "\n", 1);
>
> fixed that. Using fd's instead of FILE* was mentioned at [1]. Perhaps
> that is the direction to go in.
Oh, interesting. I'd missed your update of this thread earlier
today. It may make sense to do everything via file descriptors
directly as you suggested in your comments on that patch.
Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302
--
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] Avoid crippled getpass function on Solaris
Excerpts from Junio C Hamano's message of Sun Aug 05 21:59:48 -0400 2012: > Wouldn't > > #if solaris > #define getpass getpassphrase > #endif > > without anything else be more than sufficient? Yes, it would, but I was hoping to make it more explicit that the function getpass may be substituted with something else. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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] Avoid crippled getpass function on Solaris
On Solaris getpass() returns at most 8 characters which cripples the
credential reading for accounts using longer passwords. The alternate
function getpassphrase() was introduced in SunOS 5.6 and will return
up to 256 characters.
Ensure that git_terminal_prompt uses the more able function when
building on Solaris.
Signed-off-by: Ben Walton
---
Hi Jeff and Junio,
I considered making this minor change a few different ways but settled
on this as it seemed (to my eye) to most closely adhere to the way
other such things were done in the compatibility code. I'm entirely
open to modifying this if it's felt that there is a clearer/cleaner
way to do it.
I'd even considered making the function swap generic enough to be
driven by the build system. That seemed over the top though, given
that most systems either have a decent getpass() or don't use this
code path at all.
I've also briefly dabbled with getting Solaris to simply use the
HAVE_DEV_TTY code path but the terminal echo stuff hasn't worked
nicely for me just yet. (It reads the password with nothing echoed
but then displays the string after reading the newline.) This might
still be a better approach in the future, but for now, having long
password reading capability will still be a benefit to users on this
platform.
Thanks
-Ben
compat/terminal.c |2 +-
compat/terminal.h |9 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/compat/terminal.c b/compat/terminal.c
index 6d16c8f..e1ab536 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -75,7 +75,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
char *git_terminal_prompt(const char *prompt, int echo)
{
- return getpass(prompt);
+ return GETPASS(prompt);
}
#endif
diff --git a/compat/terminal.h b/compat/terminal.h
index 97db7cd..8d7b3f9 100644
--- a/compat/terminal.h
+++ b/compat/terminal.h
@@ -3,4 +3,13 @@
char *git_terminal_prompt(const char *prompt, int echo);
+/* getpass() returns at most 8 characters on solaris so use
+ getpassphrase() which returns up to 256. */
+# if defined (__SVR4) && defined (__sun) /* solaris */
+#define GETPASS getpassphrase
+#else
+#define GETPASS getpass
+#endif
+
+
#endif /* COMPAT_TERMINAL_H */
--
1.7.10.3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fix git-svn tests for SVN 1.7.5.
Hi Michael, > > I've fixed the git-svn tests for SVN 1.7 and tested with SVN 1.7.5. > > Thanks. git-svn is not maintained by Junio but by Eric and others on > the list. I'm cc-ing Eric and Ben Walton so they can benefit from > your work. This is fantastic. It's been on my todo list but not a priority for me. I'm glad you've taken the time to scratch this itch though. I'll try to run through this series in the next few days and I can also do some test builds on solaris to see how it plays there. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 -- 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
