Re: [PATCH] setup: suppress implicit . work-tree for bare repos

2013-03-08 Thread Jeff King
On Fri, Mar 08, 2013 at 08:44:20AM +0100, Johannes Sixt wrote:

 Am 3/8/2013 8:15, schrieb Jeff King:
  --- a/cache.h
  +++ b/cache.h
  @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int 
  mode)
   #define GIT_DIR_ENVIRONMENT GIT_DIR
   #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE
   #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE
  +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE
   #define DEFAULT_GIT_DIR_ENVIRONMENT .git
   #define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY
   #define INDEX_ENVIRONMENT GIT_INDEX_FILE
 
 This new variable needs to be added to environment.c:local_repo_env, right?

It does; I had no idea local_repo_env existed. We should add a comment
to cache.h to that effect, too.

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


Re: [PATCH] setup: suppress implicit . work-tree for bare repos

2013-03-08 Thread Jeff King
On Thu, Mar 07, 2013 at 11:54:18PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  diff --git a/cache.h b/cache.h
  index e493563..070169a 100644
  --- a/cache.h
  +++ b/cache.h
  @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int 
  mode)
   #define GIT_DIR_ENVIRONMENT GIT_DIR
   #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE
   #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE
  +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE
   #define DEFAULT_GIT_DIR_ENVIRONMENT .git
   #define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY
   #define INDEX_ENVIRONMENT GIT_INDEX_FILE
 
 Not adding any user documentation is fine (you explained why in the
 log message), but I would really prefer to have some in-code comment
 to clarify its meaning.  Is it Please do use implicit work tree
 boolean?  Is it This is the path to the work tree we have already
 figured out string?  Is it something else?  What is it used for,
 who sets it, what other codepath that will be invented in the future
 need to be careful to set it (or unset it) and how does one who
 writes that new codepath decides that he needs to do so (or
 shouldn't)?

My intent was that the commit message would be enough to explain it, but
it is a pain for a later reader to have to blame the line back to that
commit to read it. I'll re-roll with a comment.

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


Re: [PATCH] setup: suppress implicit . work-tree for bare repos

2013-03-07 Thread Johannes Sixt
Am 3/8/2013 8:15, schrieb Jeff King:
 --- a/cache.h
 +++ b/cache.h
 @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int 
 mode)
  #define GIT_DIR_ENVIRONMENT GIT_DIR
  #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE
  #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE
 +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE
  #define DEFAULT_GIT_DIR_ENVIRONMENT .git
  #define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY
  #define INDEX_ENVIRONMENT GIT_INDEX_FILE

This new variable needs to be added to environment.c:local_repo_env, right?

-- Hannes
--
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] setup: suppress implicit . work-tree for bare repos

2013-03-07 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 diff --git a/cache.h b/cache.h
 index e493563..070169a 100644
 --- a/cache.h
 +++ b/cache.h
 @@ -344,6 +344,7 @@ static inline enum object_type object_type(unsigned int 
 mode)
  #define GIT_DIR_ENVIRONMENT GIT_DIR
  #define GIT_NAMESPACE_ENVIRONMENT GIT_NAMESPACE
  #define GIT_WORK_TREE_ENVIRONMENT GIT_WORK_TREE
 +#define GIT_IMPLICIT_WORK_TREE_ENVIRONMENT GIT_IMPLICIT_WORK_TREE
  #define DEFAULT_GIT_DIR_ENVIRONMENT .git
  #define DB_ENVIRONMENT GIT_OBJECT_DIRECTORY
  #define INDEX_ENVIRONMENT GIT_INDEX_FILE

Not adding any user documentation is fine (you explained why in the
log message), but I would really prefer to have some in-code comment
to clarify its meaning.  Is it Please do use implicit work tree
boolean?  Is it This is the path to the work tree we have already
figured out string?  Is it something else?  What is it used for,
who sets it, what other codepath that will be invented in the future
need to be careful to set it (or unset it) and how does one who
writes that new codepath decides that he needs to do so (or
shouldn't)?

I would know *today* that it is a bool to affect us, after having
discovered that we are in bare and we have set GIT_DIR (so if the
end user already had GIT_DIR, we shouldn't set it ourselves), and
also our child processes, but I am not confident that I will
remember this thread 6 months down the road.
--
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