* [PATCH] test-lib: avoid full path to store test results
@ 2012-10-30 4:12 Felipe Contreras
2012-10-30 4:28 ` Jeff King
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Felipe Contreras @ 2012-10-30 4:12 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Jonathan Nieder,
Ævar Arnfjörð Bjarmason, Johannes Sixt,
Felipe Contreras
No reason to use the full path in case this is used externally.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
t/test-lib.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 514282c..5a3d665 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -389,7 +389,8 @@ test_done () {
then
test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
mkdir -p "$test_results_dir"
- test_results_path="$test_results_dir/${0%.sh}-$$.counts"
+ base=${0##*/}
+ test_results_path="$test_results_dir/${base%.sh}-$$.counts"
cat >>"$test_results_path" <<-EOF
total $test_count
--
1.8.0
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 4:12 [PATCH] test-lib: avoid full path to store test results Felipe Contreras @ 2012-10-30 4:28 ` Jeff King 2012-10-30 4:39 ` Felipe Contreras 2012-10-30 4:46 ` Jonathan Nieder 2012-10-30 6:58 ` Elia Pinto 2 siblings, 1 reply; 18+ messages in thread From: Jeff King @ 2012-10-30 4:28 UTC (permalink / raw) To: Felipe Contreras Cc: git, Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð Bjarmason, Johannes Sixt On Tue, Oct 30, 2012 at 05:12:57AM +0100, Felipe Contreras wrote: > No reason to use the full path in case this is used externally. I think it is not just "no reason to", but it is actively wrong to use a full path, as we do not take care to "mkdir -p" the intervening path components. However, this never comes up in practice, because all of the test scripts assume you are running them from the test directory (i.e., they will fail otherwise because they will not find ./test-lib.sh). Is this in support of putting remote-hg tests in contrib/? I had expected you to just put export TEST_DIRECTORY="$(pwd)/../../../t" . "$TEST_DIRECTORY/test-lib.sh" into the test script in contrib/remote-hg/t. I guess you are doing something like: cd ../../../t && ../contrib/remote-hg/t/twhatever... but the former seems much simpler to invoke (and if the goal is to get your test-results in the right place, setting TEST_OUTPUT_DIRECTORY is the best way to do that). If this is part of the remote-hg series, I'd prefer to just see it as part of the re-roll. It's much easier to evaluate it in context. Or are you really just doing: cd git/t $PWD/t0000-basic.sh I guess there is nothing wrong with that, though there is no reason not to use "./" instead of $PWD. -Peff ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 4:28 ` Jeff King @ 2012-10-30 4:39 ` Felipe Contreras 0 siblings, 0 replies; 18+ messages in thread From: Felipe Contreras @ 2012-10-30 4:39 UTC (permalink / raw) To: Jeff King Cc: git, Junio C Hamano, Jonathan Nieder, Ævar Arnfjörð, Johannes Sixt On Tue, Oct 30, 2012 at 5:28 AM, Jeff King <peff@peff.net> wrote: > On Tue, Oct 30, 2012 at 05:12:57AM +0100, Felipe Contreras wrote: > >> No reason to use the full path in case this is used externally. > > I think it is not just "no reason to", but it is actively wrong to use a > full path, as we do not take care to "mkdir -p" the intervening path > components. > > However, this never comes up in practice, because all of the test > scripts assume you are running them from the test directory (i.e., > they will fail otherwise because they will not find ./test-lib.sh). > > Is this in support of putting remote-hg tests in contrib/? I had > expected you to just put > > export TEST_DIRECTORY="$(pwd)/../../../t" > . "$TEST_DIRECTORY/test-lib.sh" If there was a single script and we didn't want reports, sure, but this is not too bad: TESTS := $(wildcard test*.sh) export T := $(addprefix $(CURDIR)/,$(TESTS)) export MAKE := $(MAKE) -e export PATH := $(CURDIR):$(PATH) test: $(MAKE) -C ../../t $@ $(TESTS): $(MAKE) -C ../../t $(CURDIR)/$@ .PHONY: $(TESTS) I just sent the new remote-hg patch series with that. Cheers. -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 4:12 [PATCH] test-lib: avoid full path to store test results Felipe Contreras 2012-10-30 4:28 ` Jeff King @ 2012-10-30 4:46 ` Jonathan Nieder 2012-10-30 16:02 ` Felipe Contreras 2012-10-30 6:58 ` Elia Pinto 2 siblings, 1 reply; 18+ messages in thread From: Jonathan Nieder @ 2012-10-30 4:46 UTC (permalink / raw) To: Felipe Contreras Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason, Johannes Sixt Hi, Felipe Contreras wrote: > No reason to use the full path in case this is used externally. > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> "No reason not to" is not a reason to do anything. What symptoms does this prevent? Could you describe the benefit of this patch in a paragraph starting "Now you can ..."? Thanks, Jonathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 4:46 ` Jonathan Nieder @ 2012-10-30 16:02 ` Felipe Contreras 2012-10-31 1:27 ` Jonathan Nieder 0 siblings, 1 reply; 18+ messages in thread From: Felipe Contreras @ 2012-10-30 16:02 UTC (permalink / raw) To: Jonathan Nieder Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt On Tue, Oct 30, 2012 at 5:46 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > Felipe Contreras wrote: > >> No reason to use the full path in case this is used externally. >> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > > "No reason not to" is not a reason to do anything. What symptoms does > this prevent? Could you describe the benefit of this patch in a > paragraph starting "Now you can ..."? ./test-lib.sh: line 394: /home/felipec/dev/git/t/test-results//home/felipec/dev/git/contrib/remote-hg/test-21865.counts: No such file or directory -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 16:02 ` Felipe Contreras @ 2012-10-31 1:27 ` Jonathan Nieder 2012-10-31 1:59 ` Felipe Contreras 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Nieder @ 2012-10-31 1:27 UTC (permalink / raw) To: Felipe Contreras Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt Felipe Contreras wrote: > On Tue, Oct 30, 2012 at 5:46 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > > Felipe Contreras wrote: >>> No reason to use the full path in case this is used externally. >>> >>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> >> >> "No reason not to" is not a reason to do anything. What symptoms does >> this prevent? Could you describe the benefit of this patch in a >> paragraph starting "Now you can ..."? > > ./test-lib.sh: line 394: > /home/felipec/dev/git/t/test-results//home/felipec/dev/git/contrib/remote-hg/test-21865.counts: > No such file or directory Ok, so a description for this patch is test: use test's basename to name results file Running a test using its full path produces an error: $ ~/dev/git/contrib/remote-hg/test-21865.sh [...] ./test-lib.sh: line 394: /home/felipec/dev/t/\ test-results//home/felipec/dev/git/contrib/remote-hg/\ test-21865.counts: No such file or directory In --tee and --valgrind modes we already use the basename to name the .out and .exit files; this patch teaches the test-lib to name the .counts file the same way. That is still not enough to tell if it is a good change, though. Should the test results for contrib/remote-hg/test-* be included with the results for t/t*.sh when I run "make aggregate-results"? Before 60d02ccc, git-svn had its own testsuite under contrib/, with glue in contrib/git-svn/t/lib-git-svn.sh to use test-lib --- maybe that code could provide some inspiration for questions like these. Hope that helps, Jonathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-31 1:27 ` Jonathan Nieder @ 2012-10-31 1:59 ` Felipe Contreras 2012-10-31 2:13 ` Jonathan Nieder 0 siblings, 1 reply; 18+ messages in thread From: Felipe Contreras @ 2012-10-31 1:59 UTC (permalink / raw) To: Jonathan Nieder Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt On Wed, Oct 31, 2012 at 2:27 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > Felipe Contreras wrote: >> On Tue, Oct 30, 2012 at 5:46 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: >> > Felipe Contreras wrote: > >>>> No reason to use the full path in case this is used externally. >>>> >>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> >>> >>> "No reason not to" is not a reason to do anything. What symptoms does >>> this prevent? Could you describe the benefit of this patch in a >>> paragraph starting "Now you can ..."? >> >> ./test-lib.sh: line 394: >> /home/felipec/dev/git/t/test-results//home/felipec/dev/git/contrib/remote-hg/test-21865.counts: >> No such file or directory > > Ok, so a description for this patch is > > test: use test's basename to name results file Is this solving an actual problem or is it just something nice to do? Like in all good novels, one has to read more find out... > Running a test using its full path produces an error: I'm not sure what that even means. Do you mean this produces an error? % make -C t $PWD/t9902-completion.sh Well, sure it does, but this patch doesn't fix that. If you want a precise explanation of what kind of usages are enabled by this patch, that would require some work, and no I haven't done it, and no, I'm not sure. > $ ~/dev/git/contrib/remote-hg/test-21865.sh > [...] > ./test-lib.sh: line 394: /home/felipec/dev/t/\ > test-results//home/felipec/dev/git/contrib/remote-hg/\ > test-21865.counts: No such file or directory Except that I didn't do this. So the fact that this happens is an assumption, and I'm not willing to make that. Most likely if somebody does that they are doing something wrong; they didn't define the TESTDIR variable (or something like that). It's all fun and games to write explanations for things, but it's not that easy when you want those explanations to be actually true, and corrent--you have to spend time to make sure of that. > In --tee and --valgrind modes we already use the basename > to name the .out and .exit files; this patch teaches the test-lib > to name the .counts file the same way. I don't see the point of listing each and every place where this already happens. As a matter of fact, the base-name is used in other places as well, and just saying "This is already done in other places", is more than enough. But who says they are not the ones doing it wrong? Maybe this part of the code is right, and it's the others that need fixing. I don't see how saying "Others are doing it" makes the patch better or worse in any way. There might also be different reasons for why they do it that doesn't apply here. > That is still not enough to tell if it is a good change, though. > Should the test results for contrib/remote-hg/test-* be included with > the results for t/t*.sh when I run "make aggregate-results"? > > Before 60d02ccc, git-svn had its own testsuite under contrib/, with > glue in contrib/git-svn/t/lib-git-svn.sh to use test-lib --- maybe > that code could provide some inspiration for questions like these. Or maybe they are the ones that should look for inspiration in contrib/remote-hg. The patch is obviously correct; it's generally good not to name files with slashes in them, and $0 is not guaranteed not to have slashes. Even if you run all the tests inside the 't' directory, this script is not only used by git, and others might want sub-directories, and not thousands of tests on the same directory like git. Either way, if obvious fixes that are one-liners require an essay for you, I give up. Cheers. -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-31 1:59 ` Felipe Contreras @ 2012-10-31 2:13 ` Jonathan Nieder 2012-10-31 2:28 ` Felipe Contreras 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Nieder @ 2012-10-31 2:13 UTC (permalink / raw) To: Felipe Contreras Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt Felipe Contreras wrote: > It's all fun and games to write explanations for things, but it's not > that easy when you want those explanations to be actually true, and > corrent--you have to spend time to make sure of that. That's why it's useful for the patch submitter to write them, asking for help when necessary. As a bonus, it helps reviewers understand the effect of the patch. Bugs averted! [...] > Either way, if obvious fixes that are one-liners require an essay for > you, I give up. I guess it is fair to call a reasonable subject line plus a couple of sentences an essay. Yes, obvious fixes especially require that, since the bug caused by an obvious fix is one of the worst kinds. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-31 2:13 ` Jonathan Nieder @ 2012-10-31 2:28 ` Felipe Contreras 2012-10-31 18:02 ` Johannes Sixt 0 siblings, 1 reply; 18+ messages in thread From: Felipe Contreras @ 2012-10-31 2:28 UTC (permalink / raw) To: Jonathan Nieder Cc: git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > Felipe Contreras wrote: > >> It's all fun and games to write explanations for things, but it's not >> that easy when you want those explanations to be actually true, and >> corrent--you have to spend time to make sure of that. > > That's why it's useful for the patch submitter to write them, asking > for help when necessary. > > As a bonus, it helps reviewers understand the effect of the patch. > Bugs averted! Yeah, that would be nice. Too bad I don't have that information, and have _zero_ motivation to go and get it for you. > [...] >> Either way, if obvious fixes that are one-liners require an essay for >> you, I give up. > > I guess it is fair to call a reasonable subject line plus a couple of > sentences an essay. Yes, obvious fixes especially require that, since > the bug caused by an obvious fix is one of the worst kinds. Yes, I've written essays for one-line fixes, in the Linux kernel, where details matter, and things are not so obvious. This is not the case here. This is as obvious and simple as things get. If there was a problem with it, somebody would have found it by now. Let not the perfect be the enemy of the good. Or do. Up to you. -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-31 2:28 ` Felipe Contreras @ 2012-10-31 18:02 ` Johannes Sixt 2012-10-31 18:28 ` Felipe Contreras 2012-11-02 13:17 ` Jeff King 0 siblings, 2 replies; 18+ messages in thread From: Johannes Sixt @ 2012-10-31 18:02 UTC (permalink / raw) To: Felipe Contreras Cc: Jonathan Nieder, git, Junio C Hamano, Jeff King, Ævar Arnfjörð Am 31.10.2012 03:28, schrieb Felipe Contreras: > On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: >> Felipe Contreras wrote: >> >>> It's all fun and games to write explanations for things, but it's not >>> that easy when you want those explanations to be actually true, and >>> corrent--you have to spend time to make sure of that. >> >> That's why it's useful for the patch submitter to write them, asking >> for help when necessary. >> >> As a bonus, it helps reviewers understand the effect of the patch. >> Bugs averted! > > Yeah, that would be nice. Too bad I don't have that information, and > have _zero_ motivation to go and get it for you. Just to clarify: That information is not just for Jonathan, but for everyone on this list and those who dig the history a year down the road. Contributors who have _zero_ motiviation to find out that information are not welcome here because they cause friction and take away time from many others for _zero_ gain. -- Hannes ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-31 18:02 ` Johannes Sixt @ 2012-10-31 18:28 ` Felipe Contreras 2012-11-02 13:17 ` Jeff King 1 sibling, 0 replies; 18+ messages in thread From: Felipe Contreras @ 2012-10-31 18:28 UTC (permalink / raw) To: Johannes Sixt Cc: Jonathan Nieder, git, Junio C Hamano, Jeff King, Ævar Arnfjörð On Wed, Oct 31, 2012 at 7:02 PM, Johannes Sixt <j6t@kdbg.org> wrote: > Am 31.10.2012 03:28, schrieb Felipe Contreras: >> Yeah, that would be nice. Too bad I don't have that information, and >> have _zero_ motivation to go and get it for you. > > Just to clarify: That information is not just for Jonathan, but for > everyone on this list and those who dig the history a year down the > road. Information that nobody has requested but Johannes. > Contributors who have _zero_ motiviation to find out that > information are not welcome here because they cause friction and take > away time from many others for _zero_ gain. Fine, stay with the broken code. Cheers. -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-31 18:02 ` Johannes Sixt 2012-10-31 18:28 ` Felipe Contreras @ 2012-11-02 13:17 ` Jeff King 2012-11-02 15:17 ` Felipe Contreras 1 sibling, 1 reply; 18+ messages in thread From: Jeff King @ 2012-11-02 13:17 UTC (permalink / raw) To: Felipe Contreras Cc: Johannes Sixt, Jonathan Nieder, git, Junio C Hamano, Ævar Arnfjörð On Wed, Oct 31, 2012 at 07:02:48PM +0100, Johannes Sixt wrote: > Am 31.10.2012 03:28, schrieb Felipe Contreras: > > On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > >> Felipe Contreras wrote: > >> > >>> It's all fun and games to write explanations for things, but it's not > >>> that easy when you want those explanations to be actually true, and > >>> corrent--you have to spend time to make sure of that. > >> > >> That's why it's useful for the patch submitter to write them, asking > >> for help when necessary. > >> > >> As a bonus, it helps reviewers understand the effect of the patch. > >> Bugs averted! > > > > Yeah, that would be nice. Too bad I don't have that information, and > > have _zero_ motivation to go and get it for you. > > Just to clarify: That information is not just for Jonathan, but for > everyone on this list and those who dig the history a year down the > road. Contributors who have _zero_ motiviation to find out that > information are not welcome here because they cause friction and take > away time from many others for _zero_ gain. And me, who is trying to figure out what to do with this patch. It is presented on its own, outside of a series, with only the description "no reason not to do this". But AFAICT, it is _required_ for the tests in the remote-hg series to work. Isn't that kind of an important motivation? Yet it is not in the commit message, nor does the remote-hg series indicate that it should be built on top. Or am I wrong that the one is dependent on the other? -Peff ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-11-02 13:17 ` Jeff King @ 2012-11-02 15:17 ` Felipe Contreras 2012-11-02 15:20 ` Jeff King 0 siblings, 1 reply; 18+ messages in thread From: Felipe Contreras @ 2012-11-02 15:17 UTC (permalink / raw) To: Jeff King Cc: Johannes Sixt, Jonathan Nieder, git, Junio C Hamano, Ævar Arnfjörð On Fri, Nov 2, 2012 at 2:17 PM, Jeff King <peff@peff.net> wrote: > On Wed, Oct 31, 2012 at 07:02:48PM +0100, Johannes Sixt wrote: > >> Am 31.10.2012 03:28, schrieb Felipe Contreras: >> > On Wed, Oct 31, 2012 at 3:13 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: >> >> Felipe Contreras wrote: >> >> >> >>> It's all fun and games to write explanations for things, but it's not >> >>> that easy when you want those explanations to be actually true, and >> >>> corrent--you have to spend time to make sure of that. >> >> >> >> That's why it's useful for the patch submitter to write them, asking >> >> for help when necessary. >> >> >> >> As a bonus, it helps reviewers understand the effect of the patch. >> >> Bugs averted! >> > >> > Yeah, that would be nice. Too bad I don't have that information, and >> > have _zero_ motivation to go and get it for you. >> >> Just to clarify: That information is not just for Jonathan, but for >> everyone on this list and those who dig the history a year down the >> road. Contributors who have _zero_ motiviation to find out that >> information are not welcome here because they cause friction and take >> away time from many others for _zero_ gain. > > And me, who is trying to figure out what to do with this patch. It is > presented on its own, outside of a series, with only the description "no > reason not to do this". Yeah, because I think it stands on its own. But I'll include it in the remote-hg patch series, I already have it queued up. > But AFAICT, it is _required_ for the tests in > the remote-hg series to work. Isn't that kind of an important > motivation? It's not _requied_, you will see that error at the end, and the aggregate results would all be 0, but the tests would still work. > Yet it is not in the commit message, nor does the remote-hg series > indicate that it should be built on top. Or am I wrong that the one is > dependent on the other? They are not dependent. Cheers. -- Felipe Contreras ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-11-02 15:17 ` Felipe Contreras @ 2012-11-02 15:20 ` Jeff King 0 siblings, 0 replies; 18+ messages in thread From: Jeff King @ 2012-11-02 15:20 UTC (permalink / raw) To: Felipe Contreras Cc: Johannes Sixt, Jonathan Nieder, git, Junio C Hamano, Ævar Arnfjörð On Fri, Nov 02, 2012 at 04:17:27PM +0100, Felipe Contreras wrote: > > And me, who is trying to figure out what to do with this patch. It is > > presented on its own, outside of a series, with only the description "no > > reason not to do this". > > Yeah, because I think it stands on its own. But I'll include it in the > remote-hg patch series, I already have it queued up. Thanks. -Peff ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 4:12 [PATCH] test-lib: avoid full path to store test results Felipe Contreras 2012-10-30 4:28 ` Jeff King 2012-10-30 4:46 ` Jonathan Nieder @ 2012-10-30 6:58 ` Elia Pinto 2012-10-30 7:01 ` Jonathan Nieder 2 siblings, 1 reply; 18+ messages in thread From: Elia Pinto @ 2012-10-30 6:58 UTC (permalink / raw) To: Felipe Contreras, git, Junio C Hamano, Jeff King, Jonathan Nieder, Ævar Arnfjörð Bjarmason, Johannes Sixt The shell word splitting done in base is a bashism, iow not portable. Best 2012/10/30, Felipe Contreras <felipe.contreras@gmail.com>: > No reason to use the full path in case this is used externally. > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com> > --- > t/test-lib.sh | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 514282c..5a3d665 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -389,7 +389,8 @@ test_done () { > then > test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results" > mkdir -p "$test_results_dir" > - test_results_path="$test_results_dir/${0%.sh}-$$.counts" > + base=${0##*/} > + test_results_path="$test_results_dir/${base%.sh}-$$.counts" > > cat >>"$test_results_path" <<-EOF > total $test_count > -- > 1.8.0 > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Inviato dal mio dispositivo mobile ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 6:58 ` Elia Pinto @ 2012-10-30 7:01 ` Jonathan Nieder 2012-10-30 22:17 ` Elia Pinto 0 siblings, 1 reply; 18+ messages in thread From: Jonathan Nieder @ 2012-10-30 7:01 UTC (permalink / raw) To: Elia Pinto Cc: Felipe Contreras, git, Junio C Hamano, Jeff King, Ævar Arnfjörð Bjarmason, Johannes Sixt Elia Pinto wrote: > The shell word splitting done in base is a bashism, iow not portable. No, ${varname##glob} is in POSIX and we already use it here and there. See Documentation/CodingGuidelines: - We use ${parameter#word} and its [#%] siblings, and their doubled "longest matching" form. Thanks for looking the patch over. Jonathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 7:01 ` Jonathan Nieder @ 2012-10-30 22:17 ` Elia Pinto 2012-10-31 9:05 ` Stefano Lattarini 0 siblings, 1 reply; 18+ messages in thread From: Elia Pinto @ 2012-10-30 22:17 UTC (permalink / raw) To: Jonathan Nieder, Felipe Contreras, git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt Thanks. I know that posix support these usages, but exists some traditional shell that not support it. These are described in the autoconf manual, last time i have checked. As the construct ; export var = x should be portable, but it is not. If this is important these days i don't know. Best 2012/10/30, Jonathan Nieder <jrnieder@gmail.com>: > Elia Pinto wrote: > >> The shell word splitting done in base is a bashism, iow not portable. > > No, ${varname##glob} is in POSIX and we already use it here and there. > See Documentation/CodingGuidelines: > > - We use ${parameter#word} and its [#%] siblings, and their > doubled "longest matching" form. > > Thanks for looking the patch over. > Jonathan > -- Inviato dal mio dispositivo mobile ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] test-lib: avoid full path to store test results 2012-10-30 22:17 ` Elia Pinto @ 2012-10-31 9:05 ` Stefano Lattarini 0 siblings, 0 replies; 18+ messages in thread From: Stefano Lattarini @ 2012-10-31 9:05 UTC (permalink / raw) To: Elia Pinto Cc: Jonathan Nieder, Felipe Contreras, git, Junio C Hamano, Jeff King, Ævar Arnfjörð, Johannes Sixt On 10/30/2012 11:17 PM, Elia Pinto wrote: > Thanks. I know that posix support these usages, but exists some > traditional shell that not support it. > True, but those shells are not POSIX shells -- the major example that comes to mind is the accursed Solaris /bin/sh. Since Git assumes a POSIX shell in its scripts and testsuite, use of any POSIX feature should be fine -- until someone can show a real-world POSIX shell that (likely due to a bug) fails to grasp such feature, in which case a "pragmatic" workaround is needed. Oh, and BTW, there are talks (and mostly consensus) among the Autotools developers to start requiring a POSIX shell in the configure scripts and Makefile recipes in the near future: <http://lists.gnu.org/archive/html/bug-autoconf/2012-06/msg00009.html> And also, related: <http://lists.gnu.org/archive/html/automake/2012-08/msg00046.html> <http://lists.gnu.org/archive/html/coreutils/2012-10/msg00127.html> >These are described in the > autoconf manual, last time i have checked. As the construct ; export > var = x should be portable, but it is not. > I don't think POSIX requires that to be portable. > If this is important these days i don't know. > I hope the above helps to clarify the matter a little. Regards, Stefano ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-11-02 15:21 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-30 4:12 [PATCH] test-lib: avoid full path to store test results Felipe Contreras 2012-10-30 4:28 ` Jeff King 2012-10-30 4:39 ` Felipe Contreras 2012-10-30 4:46 ` Jonathan Nieder 2012-10-30 16:02 ` Felipe Contreras 2012-10-31 1:27 ` Jonathan Nieder 2012-10-31 1:59 ` Felipe Contreras 2012-10-31 2:13 ` Jonathan Nieder 2012-10-31 2:28 ` Felipe Contreras 2012-10-31 18:02 ` Johannes Sixt 2012-10-31 18:28 ` Felipe Contreras 2012-11-02 13:17 ` Jeff King 2012-11-02 15:17 ` Felipe Contreras 2012-11-02 15:20 ` Jeff King 2012-10-30 6:58 ` Elia Pinto 2012-10-30 7:01 ` Jonathan Nieder 2012-10-30 22:17 ` Elia Pinto 2012-10-31 9:05 ` Stefano Lattarini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).