Re: [PATCH v2 15/16] config: add core.noshallow to prevent turning a repo into a shallow one

2013-07-23 Thread Philip Oakley 

From: "Duy Nguyen" 
Sent: Tuesday, July 23, 2013 2:28 AM
On Tue, Jul 23, 2013 at 2:23 AM, Philip Oakley  
wrote:

From: "Nguyễn Thái Ngọc Duy" 
Subject: [PATCH v2 15/16] config: add core.noshallow to prevent 
turning a

repo into a shallow one

Surely this should be the default now that it is possible to corrupt 
a
golden repo by pushing/fetching a shallow repository to it and it 
then

becomes shallow, and all the followers become shallow as well, with
consequent problems (IIUC) [PATCH v2 05/16].

It would be just as easy to change the config to core.allowshallow 
which
then must be enabled by the user, and can be mentioned in the shallow 
clone

option's documentation.


Clarification, it's not really "corrupt". If you have full history
from a ref "A", fetching from another shallow clone does not touch the
history of ref A at all



  (that is if you do _not_ specify --depth).


I hadn't appreciated this conditional.

It
may add a a shallow ref B, which is the reason the whole repo becomes
shallow.
I had read the initial commit message (in 05/16) as implying that it was 
possible to fool someone into pulling a shallow repo and that would make 
their repo just as shallow (that's without a --depth argument to the 
command). Had that been the case then it would have been possible to 
loose some data from deep in the DAG. Glad to hear I was mistaken. 
Perhaps if your comment above is included in the commit message to 
ensure that clarification is there.



The same goes for push. This is not implemented, but I'm
thinking of adding "clean .git/shallow" to git repack -ad. Then if you
delete ref B and repack -ad, the repo could become full again.

But yeah, maybe defaulting to no shallow is better. Will do so in the
reroll unless someone objects.
--
Duy
--
Philip 


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 15/16] config: add core.noshallow to prevent turning a repo into a shallow one

2013-07-22 Thread Junio C Hamano 
Duy Nguyen  writes:

> But yeah, maybe defaulting to no shallow is better. Will do so in the
> reroll unless someone objects.

No objections from me ;-).
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 15/16] config: add core.noshallow to prevent turning a repo into a shallow one

2013-07-22 Thread Duy Nguyen 
On Tue, Jul 23, 2013 at 2:23 AM, Philip Oakley  wrote:
> From: "Nguyễn Thái Ngọc Duy" 
> Subject: [PATCH v2 15/16] config: add core.noshallow to prevent turning a
> repo into a shallow one
>
> Surely this should be the default now that it is possible to corrupt a
> golden repo by pushing/fetching a shallow repository to it and it then
> becomes shallow, and all the followers become shallow as well, with
> consequent problems (IIUC) [PATCH v2 05/16].
>
> It would be just as easy to change the config to core.allowshallow which
> then must be enabled by the user, and can be mentioned in the shallow clone
> option's documentation.

Clarification, it's not really "corrupt". If you have full history
from a ref "A", fetching from another shallow clone does not touch the
history of ref A at all (that is if you do _not_ specify --depth). It
may add a a shallow ref B, which is the reason the whole repo becomes
shallow. The same goes for push. This is not implemented, but I'm
thinking of adding "clean .git/shallow" to git repack -ad. Then if you
delete ref B and repack -ad, the repo could become full again.

But yeah, maybe defaulting to no shallow is better. Will do so in the
reroll unless someone objects.
--
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 15/16] config: add core.noshallow to prevent turning a repo into a shallow one

2013-07-22 Thread Philip Oakley 

From: "Nguyễn Thái Ngọc Duy" 
Subject: [PATCH v2 15/16] config: add core.noshallow to prevent turning 
a repo into a shallow one


Surely this should be the default now that it is possible to corrupt a 
golden repo by pushing/fetching a shallow repository to it and it then 
becomes shallow, and all the followers become shallow as well, with 
consequent problems (IIUC) [PATCH v2 05/16].


It would be just as easy to change the config to core.allowshallow which 
then must be enabled by the user, and can be mentioned in the shallow 
clone option's documentation.




Signed-off-by: Nguyễn Thái Ngọc Duy 
---
Documentation/config.txt | 5 +
builtin/receive-pack.c   | 9 -
cache.h  | 1 +
config.c | 5 +
environment.c| 1 +
fetch-pack.c | 9 -
t/t5536-fetch-shallow.sh | 9 +
t/t5537-push-shallow.sh  | 6 ++
8 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 81856dd..e811180 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -655,6 +655,11 @@ core.abbrev::
 for abbreviated object names to stay unique for sufficiently long
 time.

+core.noshallow::
+ If true, reject any pushes or fetches that may turn the
+ repository into a shallow one. This setting is ignored if the
+ repository is already shallow.
+
add.ignore-errors::
add.ignoreErrors::
 Tells 'git add' to continue adding files when some files cannot be
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 54bf6b2..95ea481 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -936,7 +936,14 @@ static const char *unpack(int err_fd)
 &shallow,
 WRITE_SHALLOW_NO_CUT |
 WRITE_SHALLOW_REWRITE);
- commit_lock_file(&shallow_lock);
+ if (*alternate_shallow_file == '\0') {
+ unlink_or_warn(git_path("shallow"));
+ rollback_lock_file(&shallow_lock);
+ } else {
+ if (!is_repository_shallow() && cannot_be_shallow)
+ die("not allowed to turn this repository into a shallow one");
+ commit_lock_file(&shallow_lock);
+ }
 }
 return NULL;
}
diff --git a/cache.h b/cache.h
index 7f17228..3a52b08 100644
--- a/cache.h
+++ b/cache.h
@@ -592,6 +592,7 @@ extern int fsync_object_files;
extern int core_preload_index;
extern int core_apply_sparse_checkout;
extern int precomposed_unicode;
+extern int cannot_be_shallow;

/*
 * The character that begins a commented line in user-editable file
diff --git a/config.c b/config.c
index d04e815..31f5a57 100644
--- a/config.c
+++ b/config.c
@@ -784,6 +784,11 @@ static int git_default_core_config(const char 
*var, const char *value)

 return 0;
 }

+ if (!strcmp(var, "core.noshallow")) {
+ cannot_be_shallow = git_config_bool(var, value);
+ return 0;
+ }
+
 /* Add other config variables here and to Documentation/config.txt. 
*/

 return 0;
}
diff --git a/environment.c b/environment.c
index 0cb67b2..14c8005 100644
--- a/environment.c
+++ b/environment.c
@@ -61,6 +61,7 @@ int merge_log_config = -1;
int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() 
*/

struct startup_info *startup_info;
unsigned long pack_size_limit_cfg;
+int cannot_be_shallow;

/*
 * The character that begins a commented line in user-editable file
diff --git a/fetch-pack.c b/fetch-pack.c
index f337526..40e7aa2 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -960,7 +960,14 @@ struct ref *fetch_pack(struct fetch_pack_args 
*args,

 WRITE_SHALLOW_NO_CUT |
 WRITE_SHALLOW_REWRITE);
 }
- commit_lock_file(&shallow_lock);
+ if (*alternate_shallow_file == '\0') {
+ unlink_or_warn(git_path("shallow"));
+ rollback_lock_file(&shallow_lock);
+ } else {
+ if (!is_repository_shallow() && cannot_be_shallow)
+ die("not allowed to turn this repository into a shallow one");
+ commit_lock_file(&shallow_lock);
+ }
 }
 }

diff --git a/t/t5536-fetch-shallow.sh b/t/t5536-fetch-shallow.sh
index 6ea6347..b7f89b1 100755
--- a/t/t5536-fetch-shallow.sh
+++ b/t/t5536-fetch-shallow.sh
@@ -102,6 +102,15 @@ EOF

'

+test_expect_success 'core.noshallow' '
+ git init clean &&
+ (
+ cd clean &&
+ git config core.noshallow true &&
+ test_must_fail git fetch ../shallow/.git
+ )
+'
+
if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
 say 'skipping remaining tests, git built without http support'
 test_done
diff --git a/t/t5537-push-shallow.sh b/t/t5537-push-shallow.sh
index 8bea496..0edd51f 100755
--- a/t/t5537-push-shallow.sh
+++ b/t/t5537-push-shallow.sh
@@ -108,6 +108,12 @@ EOF
 )
'

+test_expect_success 'core.noshallow' '
+ git init clean &&
+ git --git-dir=clean/.git config core.noshallow true &&
+ test_must_fail git --git-dir=shallow/.git push clean 
master:refs/remotes/shallow/master

+'
+
if test -n "$NO_CURL" -o -z

[PATCH v2 15/16] config: add core.noshallow to prevent turning a repo into a shallow one

2013-07-20 Thread Nguyễn Thái Ngọc Duy 

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/config.txt | 5 +
 builtin/receive-pack.c   | 9 -
 cache.h  | 1 +
 config.c | 5 +
 environment.c| 1 +
 fetch-pack.c | 9 -
 t/t5536-fetch-shallow.sh | 9 +
 t/t5537-push-shallow.sh  | 6 ++
 8 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 81856dd..e811180 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -655,6 +655,11 @@ core.abbrev::
for abbreviated object names to stay unique for sufficiently long
time.
 
+core.noshallow::
+   If true, reject any pushes or fetches that may turn the
+   repository into a shallow one. This setting is ignored if the
+   repository is already shallow.
+
 add.ignore-errors::
 add.ignoreErrors::
Tells 'git add' to continue adding files when some files cannot be
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 54bf6b2..95ea481 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -936,7 +936,14 @@ static const char *unpack(int err_fd)
&shallow,
WRITE_SHALLOW_NO_CUT |
WRITE_SHALLOW_REWRITE);
-   commit_lock_file(&shallow_lock);
+   if (*alternate_shallow_file == '\0') {
+   unlink_or_warn(git_path("shallow"));
+   rollback_lock_file(&shallow_lock);
+   } else {
+   if (!is_repository_shallow() && cannot_be_shallow)
+   die("not allowed to turn this repository into a 
shallow one");
+   commit_lock_file(&shallow_lock);
+   }
}
return NULL;
 }
diff --git a/cache.h b/cache.h
index 7f17228..3a52b08 100644
--- a/cache.h
+++ b/cache.h
@@ -592,6 +592,7 @@ extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
+extern int cannot_be_shallow;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/config.c b/config.c
index d04e815..31f5a57 100644
--- a/config.c
+++ b/config.c
@@ -784,6 +784,11 @@ static int git_default_core_config(const char *var, const 
char *value)
return 0;
}
 
+   if (!strcmp(var, "core.noshallow")) {
+   cannot_be_shallow = git_config_bool(var, value);
+   return 0;
+   }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
 }
diff --git a/environment.c b/environment.c
index 0cb67b2..14c8005 100644
--- a/environment.c
+++ b/environment.c
@@ -61,6 +61,7 @@ int merge_log_config = -1;
 int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
 struct startup_info *startup_info;
 unsigned long pack_size_limit_cfg;
+int cannot_be_shallow;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/fetch-pack.c b/fetch-pack.c
index f337526..40e7aa2 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -960,7 +960,14 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
WRITE_SHALLOW_NO_CUT |
WRITE_SHALLOW_REWRITE);
}
-   commit_lock_file(&shallow_lock);
+   if (*alternate_shallow_file == '\0') {
+   unlink_or_warn(git_path("shallow"));
+   rollback_lock_file(&shallow_lock);
+   } else {
+   if (!is_repository_shallow() && 
cannot_be_shallow)
+   die("not allowed to turn this 
repository into a shallow one");
+   commit_lock_file(&shallow_lock);
+   }
}
}
 
diff --git a/t/t5536-fetch-shallow.sh b/t/t5536-fetch-shallow.sh
index 6ea6347..b7f89b1 100755
--- a/t/t5536-fetch-shallow.sh
+++ b/t/t5536-fetch-shallow.sh
@@ -102,6 +102,15 @@ EOF
 
 '
 
+test_expect_success 'core.noshallow' '
+   git init clean &&
+   (
+   cd clean &&
+   git config core.noshallow true &&
+   test_must_fail git fetch ../shallow/.git
+   )
+'
+
 if test -n "$NO_CURL" -o -z "$GIT_TEST_HTTPD"; then
say 'skipping remaining tests, git built without http support'
test_done
diff --git a/t/t5537-push-shallow.sh b/t/t5537-push-shallow.sh
index 8bea496..0edd51f 100755
--- a/t/t5537-push-shallow.sh
+++ b/t/t5537-push-shallow.sh
@@ -108,6 +108,12 @@ EOF
)
 '
 
+test_expect_success 'core.noshallow' '
+   git init clean &&
+   git --git-dir=clean/.git config core.noshallow true &&
+   test_must_fail git --git-dir=shallow/.git push cle