login
Search
Hmm. I am not convinced.
Setting autocrlf for every repository limits the user's options to
override the default. Currently we provide a global default and the
user can either override globally for all his repositories or on
a per-repository basis. Hence, users can decide that they want autocrlf
to never happen and can easily set this in ~/.gitconfig. If we stored
autocrlf in every newly created repository, the user would need to
override our default again and again for every new repository.
Maybe what we want is to conserve the setup that exists when a new
repository is created. Changing autocrlf later is tricky because the
work tree's line endings depend on the settings during checkout.
Therefore, it makes sense to preserve the value of autocrlf that exists
during the first checkout. In this regards autocrlf is different from,
for example, author because author can be easily changed later without
requiring any conversion of existing files in the work tree.
Patch follows.
Unfortunately the proposed change won't change the fact that existing
msysgit setups still break. I still do not see an easy way to avoid
this.
Steffen
--
Storing the current value of autocrlf to preserve it for this repository
even if the global setup changes is a good idea. Changing autocrlf
later is tricky because the work tree's line endings depend on the
settings during checkout. Therefore, it makes sense to preserve the
value of autocrlf that exists during the first checkout. In this
regards autocrlf is different from, for example, author, because author
can be easily changed later without requiring any conversion of existing
files in the work tree.
This commit modifies the initialization of a new repository to store the
current value of autocrlf.
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
builtin-init-db.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 79eaf8d..6c49a82 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -268,6 +268,22 @@ static int create_default_files(const char *git_dir, const char *template_path)
git_config_set("core.symlinks", "false");
}
+ /*
+ * Store current value of auto_crlf to preserve it for
+ * this repository even if the global setup changes.
+ */
+ switch (auto_crlf) {
+ case -1:
+ git_config_set("core.autocrlf", "input");
+ break;
+ case 0:
+ git_config_set("core.autocrlf", "false");
+ break;
+ case 1:
+ git_config_set("core.autocrlf", "true");
+ break;
+ }
+
return reinit;
}
--
1.5.5.rc0.21.g740fd.dirty
--
NAK While I agree that preserving autocrlf may be a good idea, I don't like that the idea of making an exception for autocrlf and treating the global settings for it differently than for other variables -- as something that should be copied on init. We have templates for that, so autocrlf should be placed into templates/config and then it will be automatically copied when a new repository is created. I have tested that now, and it works. Dmitry --
Hi, Then maybe a way for the user to override the global templates is what we need? I can see that this would be useful outside of the crlf issue. Ciao, Dscho --
I do not think we need this. autocrlf is a configurable variable and we
already have a mechanism for the user to override configuration
variables. The user can use "git config --global ..." to sets his
preferences. This mechanism is well established. I do not see a reason
not to use it.
Steffen
--
Hi, The point is: if we use /etc/gitconfig, we also touch _existing_ setups (as Junio pointed out). Which, in the case of autocrlf, is not desirable. And if we go the route via templates, $HOME/.gitconfig will do no good, as the configuration in the repository trumps the --global configuration. Ciao, Dscho --
I proposed a mechanism that would avoid such problems in the future.
Repositories that would copy the current setting of autocrlf to their
local .git/config would be shielded from future changes to the global
We have two conflicting objectives and I do not know how to meet them
the same time:
1) Existing setups should not break.
2) Users should have a way to set a global default for autocrlf that
overrides our defaults.
If we store the new default in newly created repositories, (1) would be
met but (2) is not possible. If we support ~/.gitconfig for overriding
the system-wide default, (2) is trivial but (1) is hard to meet.
I propose to break existing setup and provide a simple mechanism to
avoid such breakages in the future. As you pointed out earlier we are
still only releasing "previews" on Windows and one reason for this
decision was the lacking support for autocrlf. We always knew that some
work would be needed before we have a sane setup on Windows.
Maybe we can improve the installer to warn the users that the default
has changed and existing repositories must either be converted or the
global default must be overridden. The installer could ask the user to
confirm this change. Maybe this is sufficient to avoid further
complains about weird behavior after upgrading.
Steffen
--
Hi, Yes, but your solution feels a bit limited and "hot-needled" for just one Maybe. My experience is that people do not even read the big red warnings in the installer. Whatever. Ciao, Dscho --
What limits do you mean (except that it does still break existing
msysgit setups; but would avoid this problem in the future)?
Steffen
--
Hi, I have the impression that a problem just like this will arise again, just not with corelf, but with another setting that the admin might want to set per default in git-init, but the user might want to override. Ciao, Dscho --
My patch does not set the default in git-init, but only stores the current choice of autocrlf in the repository's config. autocrlf is special because it cannot be easily changed after the initial checkout. Changing autocrlf might require converting the line endings of all files in the work tree. This is very different from most other configuration variables, which can be changed without requiring any modifications to the work tree. In some sense you cannot change autocrlf at all after a repository is initialized. At least you need a deep understanding of the autocrlf mechanism to avoid unexpected behavior. Once you checked out files, these files might 'preserve' the setting of autocrlf during the checkout. For example if you use autocrlf=true during checkout and later change to autocrlf=false, git will detect all files as changed because they contain CRLF in the workspace but LF in the repository. The situation is even worse, because the choice of autocrlf is not a purely local one, but depends on the project you are cloning. A sane policy is to have only LF line-endings in the repository and use native line-endings in the work tree. This can be guaranteed with autocrlf=input on Unix and autocrlf=true on Windows. Another possible choice is to tell git to not modify any content at all, which autocrlf=false would give you. Personally, I do not think the latter is a reasonable choice, but other might have a different opinion. My point is that it is the choice of the project: Either *all* repositories that clone the project should set autocrlf=input|true or *all* repositories should set autocrlf=false. It is neither the choice of the local admin, nor the local user, nor the default of git. You need to have project-specific knowledge to make the right choice. I suspect that a lot of projects implicitly chose autocrlf=false because of lacking autocrlf support in the past. The default of git was to not touch the content at all; and still this is the default on Unix. ...
Hi, But basically all clean/smudge filters have exactly the same problem! And core.ignorecase, too! There must be a way to do it elegantly, without catering just for autocrlf. Ciao, Dscho --
To meet both requirements you have to copy autocrlf setting for each new repository during git init or clone and should never use the global value for any other purpose than this. This means that you do NOT really need the system or global default for autocrlf in the sense we have it for other configuration variables, but you need a template value for it. We already have templates for different hooks, info/exclude, etc. So, instead of placing autocrlf in /etc/gitconfig, you should place this variable to /usr/share/git/templates/config and this file should be copied by git init or git clone as any other file in templates. Dmitry --
Hi, I thought we discussed that already? And the consensus was that this does not allow for per-user overriding. Ciao, Dscho --
I am sorry I missed this discussion. In this case, I believe that the idea of templates should be extended, so any user may have his/her own templates in $HOME/.gittemplates. IMHO, it makes much more sense than making an exception for autocrlf in builtin-init-db.c and breaking existing repositories... Dmitry --
Hi, I think I actually suggested something like that. But that gets only even more complicated: if you have a template for .git/config in $HOME/.gittemplates/, then the global template will be _disregarded_, even if the administrator puts something vital in there. Maybe the best idea would be an "init" hook, settable from the config, after all. Ciao, Dscho --
Maybe, because I cannot imagine what so vital the administrator can put in it, I really do not see that as a problem. I believe that any good administrator should notify all users about any vital changes anyway, because if these changes are vital, it may be necessary not only change templates in $HOME/.gittemplates/ (which very few users will probably I agree that seems to be the best solution as it is more flexible than having $HOME/.gittemplates/. Dmitry --
This variable, if set, specifies the path of a hook which is executed after every git-init. It can be used to override settings in the templates per-user. For example, when the adminstrator set core.autoCRLF=true in the templates, but you want to avoid that explicitely. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- On Mon, 24 Mar 2008, Dmitry Potapov wrote: > On Mon, Mar 24, 2008 at 11:57:19AM +0100, Johannes Schindelin > wrote: > > > > Maybe the best idea would be an "init" hook, settable from the > > config, after all. > > I agree that seems to be the best solution as it is more > flexible than having $HOME/.gittemplates/. Documentation/config.txt | 4 ++++ Documentation/git-init.txt | 3 +++ builtin-init-db.c | 26 ++++++++++++++++++++++++++ t/t0001-init.sh | 21 +++++++++++++++++++++ 4 files changed, 54 insertions(+), 0 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index efde54d..3323724 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -358,6 +358,10 @@ core.whitespace:: does not trigger if the character before such a carriage-return is not a whitespace (not enabled by default). +core.inithook:: + The path to a program which is run after each call to + linkgit:git-init[1]. + alias.*:: Command aliases for the linkgit:git[1] command wrapper - e.g. after defining "alias.last = cat-file commit HEAD", the invocation diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt index 03a4f0e..e082218 100644 --- a/Documentation/git-init.txt +++ b/Documentation/git-init.txt @@ -60,6 +60,9 @@ If you are initializing a non-bare repository, the config variable `receive.guardCurrentBranch` is set to true. This avoids problems with pushing into the current branch, which does not touch the working tree. +If you want to run a specific script everytime git-init was issued, you +can set the variable ...
Earlier, git-init tested for a valid HEAD ref, but if the repository
was empty, there was none. Instead, test for the existence of
the file $GIT_DIR/HEAD.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
I realised this while testing the core.initHook patch.
builtin-init-db.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 1975910..ceb4727 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -168,10 +168,9 @@ static int create_default_files(const char *git_dir, const char *template_path)
{
unsigned len = strlen(git_dir);
static char path[PATH_MAX];
- unsigned char sha1[20];
struct stat st1;
char repo_version_string[10];
int reinit;
int filemode;
if (len > sizeof(path)-50)
@@ -220,7 +219,7 @@ static int create_default_files(const char *git_dir, const char *template_path)
* branch, if it does not exist yet.
*/
strcpy(path + len, "HEAD");
- reinit = !read_ref("HEAD", sha1);
+ reinit = !access(path, R_OK);
if (!reinit) {
if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
exit(1);
--
1.5.5.rc1.178.gd799d
--
If this is a HEAD (or .git/HEAD) that is a dangling symlink (we do still
support them, don't we?) wouldn't this access fail?
-- >8 --
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Date: Mon, 24 Mar 2008 16:14:52 +0100
Subject: [PATCH] init: show "Reinit" message even in an (existing) empty repository
Earlier, git-init tested for a valid HEAD ref, but if the repository
was empty, there was none. Instead, test for the existence of
the file $GIT_DIR/HEAD.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-init-db.c | 5 +++--
t/t0001-init.sh | 17 +++++++++++++++++
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 79eaf8d..2854868 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -167,9 +167,9 @@ static int create_default_files(const char *git_dir, const char *template_path)
{
unsigned len = strlen(git_dir);
static char path[PATH_MAX];
- unsigned char sha1[20];
struct stat st1;
char repo_version_string[10];
+ char junk[2];
int reinit;
int filemode;
@@ -219,7 +219,8 @@ static int create_default_files(const char *git_dir, const char *template_path)
* branch, if it does not exist yet.
*/
strcpy(path + len, "HEAD");
- reinit = !read_ref("HEAD", sha1);
+ reinit = (!access(path, R_OK)
+ || readlink(path, junk, sizeof(junk)-1) != -1);
if (!reinit) {
if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
exit(1);
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index c015405..b0289e3 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -113,4 +113,21 @@ test_expect_success 'GIT_DIR & GIT_WORK_TREE (2)' '
fi
'
+test_expect_success 'reinit' '
+
+ (
+ unset GIT_CONFIG GIT_WORK_TREE GIT_CONFIG
+
+ mkdir again &&
+ cd again &&
+ git init >out1 2>err1 &&
+ git init >out2 2>err2
+ ) &&
+ grep "Initialized empty" again/out1 &&
+ grep "Reinitialized ...Hi, Heh, yes. I originally set reinit = 0, and thought I was clever by patch editing. But I have less experience with that than necessary, and then, I Right. I already saw your commit, and while I would have to think hard if it is implementing the correct logic (what does access() do with a dangling symlink? Is || correct?), I just trust you. Although I have to admit that having the authorship of this commit is giving me more credit than I deserve. Thanks, Dscho --
... in your $HOME/.gitconfig, naturally ;-) I like the general idea, but with a few nits. The hook might want to differentiate its behaviour based on how git-init was invoked, don't you think? E.g. was it because the end user wanted to create a new repo? Re-init? Clone, cvsimport or some other higher level commands invoked git-init on the user's behalf?). The higher level ones could communicate it via environment so we do not have to worry about them too much (except perhaps in the documentation part to help hook writers), but at least re-init is something init-db alone would be able to tell the hook, so either a parameter to the hook or an environment exported to it would be needed. The hook can figure out 'shared' and 'bare' by reading from $GIT_DIR/config itself, but are there other things we may want to The current way to spell this is with git_config_string() to protect yourself from segfaulting on: [core] --
This variable, if set, specifies the path of a hook which is executed
after every git-init. It can be used to override settings in the
templates per-user.
In case of a re-initialization, the hook gets called with the argument
"reinit".
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Mon, 24 Mar 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > If you want to run a specific script everytime git-init was
> > issued, you can set the variable `core.initHook`.
>
> ... in your $HOME/.gitconfig, naturally ;-)
Naturally.
> I like the general idea, but with a few nits.
>
> The hook might want to differentiate its behaviour based on how
> git-init was invoked, don't you think?
Okay. It now gets an argument "reinit" when reinitialised. As
you said, the other usages (clone, cvsimport, ...) are a bit
fiddly to implement.
> > +static int config_init_hook(const char *key, const char *value)
> > +{
> > + if (!strcmp(key, "core.inithook"))
> > + init_hook = xstrdup(value);
> > + return 0;
>
> The current way to spell this is with git_config_string() to
> protect yourself from segfaulting on:
Yeah, sorry. Fixed.
> > @@ -407,6 +430,9 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
> > git_config_set("receive.denyNonFastforwards", "true");
> > }
> >
> > + if (run_init_hook())
> > + return 1;
> > +
>
> Hmm. Exit without a message even under !quiet?
Even under quiet, run_init_hook() will complain if the hook is
invalid.
Documentation/config.txt | 5 +++++
Documentation/git-init.txt | 6 ++++++
builtin-init-db.c | 28 ++++++++++++++++++++++++++++
t/t0001-init.sh | 21 +++++++++++++++++++++
4 files changed, 60 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index efde54d..355f049 100644
--- a/Documentation/config.txt
+++ ...Sorry, I may have been unclear but I was wondering more about the case the hook script errored out silently. --
This variable, if set, specifies the path of a hook which is executed
after every git-init. It can be used to override settings in the
templates per-user.
In case of a re-initialization, the hook gets called with the argument
"reinit".
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Ooops. You're right. This is the interdiff:
diff --git a/builtin-init-db.c b/builtin-init-db.c
index e1a54b5..cdeb1d7 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -332,7 +332,8 @@ static int run_init_hook(int reinit)
argv[0] = init_hook;
if (reinit)
argv[1] = "reinit";
- return run_command_v_opt(argv, 0);
+ return run_command_v_opt(argv, 0) ?
+ error("hook '%s' failed", init_hook) : 0;
}
Documentation/config.txt | 5 +++++
Documentation/git-init.txt | 6 ++++++
builtin-init-db.c | 29 +++++++++++++++++++++++++++++
t/t0001-init.sh | 21 +++++++++++++++++++++
4 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index efde54d..355f049 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -358,6 +358,11 @@ core.whitespace::
does not trigger if the character before such a carriage-return
is not a whitespace (not enabled by default).
+core.inithook::
+ The path to a program which is run after each call to
+ linkgit:git-init[1]. The hook is called with the argument
+ "reinit" if an existing repository is re-initialized.
+
alias.*::
Command aliases for the linkgit:git[1] command wrapper - e.g.
after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 03a4f0e..26deaaf 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -60,6 +60,12 @@ If you are initializing a non-bare repository, the config variable
`receive.guardCurrentBranch` is set to true. This ...
| Greg KH | Og dreams of kernels |
| Jens Axboe | [PATCH 31/33] Fusion: sg chaining support |
| Arnd Bergmann | Re: finding your own dead "CONFIG_" variables |
| Mark Brown | [PATCH 2/2] Subject: natsemi: Allow users to disable workaround for DspCfg reset |
| Tony Breeds | [LGUEST] Look in object dir for .config |
git: | |
| Brian Downing | Re: Git in a Nutshell guide |
| John Benes | Re: master has some toys |
| Matthias Lederhofer | [PATCH 4/7] introduce GIT_WORK_TREE to specify the work tree |
| Alexander Sulfrian | [RFC/PATCH] RE: git calls SSH_ASKPASS even if DISPLAY is not set |
| Junio C Hamano | Re: Rss produced by git is not valid xml? |
| Linux Kernel Mailing List | iSeries: fix section mismatch in iseries_veth |
| Linux Kernel Mailing List | ixbge: remove TX lock and redo TX accounting. |
| Linux Kernel Mailing List | ixgbe: fix several counter register errata |
| Linux Kernel Mailing List | b43: fix build with CONFIG_SSB_PCIHOST=n |
| Linux Kernel Mailing List | 9p: block-based virtio client |
| Michael |