[GSoC][PATCH 0/1] Introduction & microproject

Good morning, 

I intend to apply to git for this year's Google Summer of Code. I am a
Computer Science student at Gothenburg University working toward a 
master's degree. Last year I completed GSoC with LibreOffice [1].

Working on git appeals to me due to its widespread usage and the number
of people who will benefit from my contributions.

I read the introductory material, picked a microproject, looked at
previous commits and produced the patch below.

It appears that some of the listed project ideas[2] are "taken" or at
least being looked at, so I'm also looking at issues on GitGitGadget with
the label possible-gsoc-project.

Regards, 
Rasmus

Rasmus Jonsson (1):
  t1050: clean up checks for file existence

 t/t1050-large.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
2.20.1

Replace "test -f" with test_path_is_file, which gives more verbose
and accurate output.

Signed-off-by: Rasmus Jonsson <redacted>
---
 t/t1050-large.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index d3b2adb28b..667fc2a745 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh

@@ -53,7 +53,8 @@ test_expect_success 'add a large file or two' '
 	for p in .git/objects/pack/pack-*.pack
 	do
 		count=$(( $count + 1 ))
-		if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx"
+		if test_path_is_file "$p" && idx=${p%.pack}.idx &&
+		   test_path_is_file "$idx"
 		then
 			continue
 		fi

@@ -65,7 +66,7 @@ test_expect_success 'add a large file or two' '
 	test $cnt = 2 &&
 	for l in .git/objects/??/??????????????????????????????????????
 	do
-		test -f "$l" || continue
+		test_path_is_file "$l" || continue
 		bad=t
 	done &&
 	test -z "$bad" &&

@@ -76,7 +77,8 @@ test_expect_success 'add a large file or two' '
 	for p in .git/objects/pack/pack-*.pack
 	do
 		count=$(( $count + 1 ))
-		if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx"
+		if test_path_is_file "$p" && idx=${p%.pack}.idx &&
+		   test_path_is_file "$idx"
 		then
 			continue
 		fi

@@ -111,7 +113,7 @@ test_expect_success 'packsize limit' '
 		count=0 &&
 		for pi in .git/objects/pack/pack-*.idx
 		do
-			test -f "$pi" && count=$(( $count + 1 ))
+			test_path_is_file "$pi" && count=$(( $count + 1 ))
 		done &&
 		test $count = 2 &&
 

-- 
2.20.1

On Sat, Feb 22, 2020 at 08:13:35AM +0100, Rasmus Jonsson wrote:

I was confused at first why these tests use "continue", since it seems
like these conditions would be errors that could cause a test failure
(and if they're not, we probably wouldn't want to use test_path_is_file,
since it's purpose is to complain noisily).

But the part that didn't quite make it into the diff context is
something like this:

  for p in ...
    if test -f ...
    then
      continue
    fi
    bad=t
  done &&
  test -z "$bad"

I think this could be written more clearly as:

  for p in ...
    test -f ... || return 1
  done

We explicitly run the test snippets in a shell function to allow this
kind of early return.

That's orthogonal to your patch, but it might be worth doing on top, or
as a preparatory patch.

But there's one more interesting bit. The loose-object loop from the
next hunk does this:

  for l in ...
    test -f "$l" || continue
    bad=t
  done &&
  test -z "$bad"

In other words, it's checking the opposite case: the test fails if the
file _does_ exist. And so it seems like using test_path_is_file would be
the wrong thing there (it would complain noisily in the success case,
and not at all in the failure case).

I suspect this could be written more clearly by looking at the output of
`git count-objects`, or perhaps just:

  {
    # ignore exit code; will fail when the glob matches nothing
    find objects/??/ -type f >loose-objects
    test_must_be_empty loose-objects
  }

either of which would solve the "match a loose object with any length"
problem that Junio brought up.

-Peff

Use test_path_is_file() instead of 'test -f' for better debugging
information.

Signed-off-by: Rasmus Jonsson <redacted>
---

Improved formatting of the code as suggested and corrected the commit
message.

 t/t1050-large.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index d3b2adb28b..184b479a21 100755
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh

@@ -53,7 +53,8 @@ test_expect_success 'add a large file or two' '
 	for p in .git/objects/pack/pack-*.pack
 	do
 		count=$(( $count + 1 ))
-		if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx"
+		if test_path_is_file "$p" &&
+		   idx=${p%.pack}.idx && test_path_is_file "$idx"
 		then
 			continue
 		fi

@@ -65,7 +66,7 @@ test_expect_success 'add a large file or two' '
 	test $cnt = 2 &&
 	for l in .git/objects/??/??????????????????????????????????????
 	do
-		test -f "$l" || continue
+		test_path_is_file "$l" || continue
 		bad=t
 	done &&
 	test -z "$bad" &&

@@ -76,7 +77,8 @@ test_expect_success 'add a large file or two' '
 	for p in .git/objects/pack/pack-*.pack
 	do
 		count=$(( $count + 1 ))
-		if test -f "$p" && idx=${p%.pack}.idx && test -f "$idx"
+		if test_path_is_file "$p" &&
+		   idx=${p%.pack}.idx && test_path_is_file "$idx"
 		then
 			continue
 		fi

@@ -111,7 +113,7 @@ test_expect_success 'packsize limit' '
 		count=0 &&
 		for pi in .git/objects/pack/pack-*.idx
 		do
-			test -f "$pi" && count=$(( $count + 1 ))
+			test_path_is_file "$pi" && count=$(( $count + 1 ))
 		done &&
 		test $count = 2 &&
 

-- 
2.20.1