* Re: git.wiki.kernel.org spam ...
From: Johannes Schindelin @ 2013-01-04 23:27 UTC (permalink / raw)
To: rupert THURNER; +Cc: git
In-Reply-To: <CAJs9aZ_eL1jR=GqxUEy3vEWbMz6kEYOHb7pZkZWFh6yXXSx-Jg@mail.gmail.com>
Hi Rupert,
On Sat, 5 Jan 2013, rupert THURNER wrote:
> 2012/12/31 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> there are 3 admins:
> * https://git.wiki.kernel.org/index.php/Special:Contributions/KorgWikiSysop,
> last contribution january 2010
> * https://git.wiki.kernel.org/index.php/Special:Contributions/Warthog9,
> last contribution august 2010
> * https://git.wiki.kernel.org/index.php/Special:Contributions/Dscho,
> last contribution august 2010
>
> and you were (by far) the most active.
I was. John Hawley trusted me when I asked for admin privileges to keep
the spam at bay, but a very vocal voice on the mailing list tried to
discredit my work, and in the wake of the ensuing mailing list thread I
got the impression that that feeling was universal, so I abided and
stopped.
> this leaves me a little confused. who would be then be responsible? who
> would be responsible for upgrading / installing anything at the wiki?
That would be John Hawley.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Anand Kumria @ 2013-01-04 15:47 UTC (permalink / raw)
To: git; +Cc: Anand Kumria
By selecting a tag within gitk you can display information about it.
This information is output by using the command
'git cat-file tag <tagid>'
This outputs the *raw* information from the tag, amongst which is the
time - in seconds since the epoch. As useful as that value is, I find it
a lot easier to read and process time which it is something like:
"Mon Dec 31 14:26:11 2012 -0800"
This change will modify the display of tags in gitk like so:
@@ -1,7 +1,7 @@
object 5d417842efeafb6e109db7574196901c4e95d273
type commit
tag v1.8.1
-tagger Junio C Hamano <gitster@pobox.com> 1356992771 -0800
+tagger Junio C Hamano <gitster@pobox.com> Mon Dec 31 14:26:11 2012 -0800
Git 1.8.1
-----BEGIN PGP SIGNATURE-----
Signed-off-by: Anand Kumria <wildfire@progsoc.org>
---
gitk-git/gitk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..aae1c58 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -10675,7 +10675,7 @@ proc showtag {tag isnew} {
set linknum 0
if {![info exists cached_tagcontent($tag)]} {
catch {
- set cached_tagcontent($tag) [exec git cat-file tag $tag]
+ set cached_tagcontent($tag) [exec git cat-file -p $tag]
}
}
if {[info exists cached_tagcontent($tag)]} {
--
1.7.9.5
^ permalink raw reply related
* Re: git.wiki.kernel.org spam ...
From: Theodore Ts'o @ 2013-01-04 23:47 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: rupert THURNER, git
In-Reply-To: <alpine.DEB.1.00.1301050022000.32206@s15462909.onlinehome-server.info>
On Sat, Jan 05, 2013 at 12:27:12AM +0100, Johannes Schindelin wrote:
>
> I was. John Hawley trusted me when I asked for admin privileges to keep
> the spam at bay, but a very vocal voice on the mailing list tried to
> discredit my work, and in the wake of the ensuing mailing list thread I
> got the impression that that feeling was universal, so I abided and
> stopped.
>
> > this leaves me a little confused. who would be then be responsible? who
> > would be responsible for upgrading / installing anything at the wiki?
>
> That would be John Hawley.
John is one of the Linux Foundation staff members that are responsible
for the system administration of wiki.kernel.org (and kernel.org, and
bugzilla.kernel.org, etc.) They are *not* responsible for the
contents of the *.wiki.kernel.org; someone from the project has to be
the wiki maintainer.
(Note: the *.wiki.kernel.org infrastructure was originally set up at
my request, and the first such hosted wiki was ext4.wiki.kernel.org;
the second was rt.wiki.kernel.org, for which I was also the primary
wiki administrator initially. I'm confident the policy on this hasn't
changed since those early days because LF sysadmins (e.g., John and
Konstantin) do *not* have time to police the various wikis for
spam....)
- Ted
^ permalink raw reply
* Re: [PATCH] gitk: Display the date of a tag in a human friendly way.
From: Junio C Hamano @ 2013-01-04 23:50 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Anand Kumria
In-Reply-To: <1357314431-32710-1-git-send-email-wildfire@progsoc.org>
Anand Kumria <wildfire@progsoc.org> writes:
> By selecting a tag within gitk you can display information about it.
> This information is output by using the command
>
> 'git cat-file tag <tagid>'
>
> This outputs the *raw* information from the tag, amongst which is the
> time - in seconds since the epoch. As useful as that value is, I find it
> a lot easier to read and process time which it is something like:
>
> "Mon Dec 31 14:26:11 2012 -0800"
>
> This change will modify the display of tags in gitk like so:
>
> @@ -1,7 +1,7 @@
> object 5d417842efeafb6e109db7574196901c4e95d273
> type commit
> tag v1.8.1
> -tagger Junio C Hamano <gitster@pobox.com> 1356992771 -0800
> +tagger Junio C Hamano <gitster@pobox.com> Mon Dec 31 14:26:11 2012 -0800
>
> Git 1.8.1
> -----BEGIN PGP SIGNATURE-----
>
> Signed-off-by: Anand Kumria <wildfire@progsoc.org>
> ---
Sounds like a sensible thing to do but I didn't check how else
(other than purely for displaying) this string is used.
Paul, the patch is not made against your tree, so if you choose to
take it you would need to strip the leading directory at the top.
Thanks.
PS. I haven't received a pull request from you for a while; are
there accumulated changes I should be pulling in before -rc0 of the
next release we are working on?
> gitk-git/gitk | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index d93bd99..aae1c58 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -10675,7 +10675,7 @@ proc showtag {tag isnew} {
> set linknum 0
> if {![info exists cached_tagcontent($tag)]} {
> catch {
> - set cached_tagcontent($tag) [exec git cat-file tag $tag]
> + set cached_tagcontent($tag) [exec git cat-file -p $tag]
> }
> }
> if {[info exists cached_tagcontent($tag)]} {
^ permalink raw reply
* [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: David Michael @ 2013-01-05 0:35 UTC (permalink / raw)
To: git@vger.kernel.org, Junio C Hamano
It is possible for this pointer of the GIT_DIR environment variable to
survive unduplicated until further getenv calls are made. The standards
allow for subsequent calls of getenv to overwrite the string located at
its returned pointer, and this can result in broken git operations on
certain platforms.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
---
I have encountered an issue with consecutive calls to getenv
overwriting earlier values. Most notably, it prevents a plain "git
clone" from working.
Long story short: This value of GIT_DIR gets passed around setup.c
until it reaches check_repository_format_gently. This function calls
git_config_early, which eventually runs getenv("HOME"). When it
returns back to check_repository_format_gently, the gitdir variable
contains my home directory path. The end result is that I wind up
with ~/objects/ etc. and a failed repository clone. (Simply adding a
bare getenv("GIT_DIR") afterwards to reset the pointer also corrects
the problem.)
Since other platforms are apparently working, yet this getenv behavior
is supported by the standards, I am left wondering if this could be a
symptom of something else being broken on my platform (z/OS). Can
anyone more familiar with this part of git identify any condition that
obviously should not be occurring?
Thanks.
setup.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/setup.c b/setup.c
index f108c4b..64fb160 100644
--- a/setup.c
+++ b/setup.c
@@ -675,8 +675,12 @@ static const char
*setup_git_directory_gently_1(int *nongit_ok)
* validation.
*/
gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
- if (gitdirenv)
- return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+ if (gitdirenv) {
+ gitdirenv = xstrdup(gitdirenv);
+ ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
+ free(gitdirenv);
+ return ret;
+ }
if (env_ceiling_dirs) {
string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
--
1.7.11.7
^ permalink raw reply related
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 1:17 UTC (permalink / raw)
To: David Michael; +Cc: git@vger.kernel.org
In-Reply-To: <CAEvUa7niTJVfp8_kuWs50kvhfZ59F-yAuAmeOXEduHXOq-tRFA@mail.gmail.com>
David Michael <fedora.dm0@gmail.com> writes:
> I have encountered an issue with consecutive calls to getenv
> overwriting earlier values. Most notably, it prevents a plain "git
> clone" from working.
>
> Long story short: This value of GIT_DIR gets passed around setup.c
> until it reaches check_repository_format_gently. This function calls
> git_config_early, which eventually runs getenv("HOME"). When it
> returns back to check_repository_format_gently, the gitdir variable
> contains my home directory path. The end result is that I wind up
> with ~/objects/ etc. and a failed repository clone. (Simply adding a
> bare getenv("GIT_DIR") afterwards to reset the pointer also corrects
> the problem.)
>
> Since other platforms are apparently working, yet this getenv behavior
> is supported by the standards, I am left wondering if this could be a
> symptom of something else being broken on my platform (z/OS).
The execve(2) function
int execve(const char *filename, char *const argv[],
char *const envp[]);
takes a NULL terminated array of NUL terminated strings of form
"VAR=VAL" in envp[], and this is kept in:
extern char **environ;
of the new image that runs.
The most naive and straight-forward way to implement getenv(3) is to
iterate over this environ[] array to look for an element that begins
with "GIT_DIR=", and return the pointer pointing at the location one
byte past that equal sign. So even if the standard allowed the
returned value to be volatile across calls to getenv(3), it will
take *more* work for implementations if they want to break our use
pattern. They have to deliberately return a string that they will
overwrite in subsequent calls to getenv(3).
Also the natural way to implement putenv(3) and setenv(3) is to
replace the pointer in the environ[] array (not overwrite the
existing string in environ[] that holds the "VAR=VAL" string that
represents the current value, which might be shorter than the new
value of the enviornment variable), hence even calling these
functions is unlikely to invalidate the result you previously
received from getenv(3).
I am not at all surprised that nobody from other platforms has seen
this breakage.
In fact,
http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
says that only setenv(), unsetenv() and putenv() may invalidate
previous return values. Note that getenv() is not listed as a
function that is allowed to break return values from a previous call
to getenv().
So in short, your platform's getenv(3) emulation may be broken. We
have other calls to getenv() where we rely on the result of it being
stable, and you might discover these places also break.
Having said that, we do have codepaths to update a handful of
environment variables ourselves (GIT_DIR is among them), so I think
your patch is a good safety measure in general.
> setup.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index f108c4b..64fb160 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -675,8 +675,12 @@ static const char
> *setup_git_directory_gently_1(int *nongit_ok)
> * validation.
> */
> gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
> - if (gitdirenv)
> - return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + if (gitdirenv) {
> + gitdirenv = xstrdup(gitdirenv);
> + ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + free(gitdirenv);
> + return ret;
> + }
>
> if (env_ceiling_dirs) {
> string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
> --
> 1.7.11.7
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: David Michael @ 2013-01-05 2:15 UTC (permalink / raw)
To: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <7vsj6gqvhc.fsf@alter.siamese.dyndns.org>
Hi,
On Fri, Jan 4, 2013 at 8:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> In fact,
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
>
> says that only setenv(), unsetenv() and putenv() may invalidate
> previous return values. Note that getenv() is not listed as a
> function that is allowed to break return values from a previous call
> to getenv().
Before I sent the e-mail, I checked that very page to be sure I wasn't
entirely insane. Specifically, the second paragraph begins with:
> The string pointed to may be overwritten by a subsequent call to getenv(), [...]
I read that line as confirmation that this is indeed acceptably
standard behavior. Even the getenv man page on my Fedora workstation
says:
> The string pointed to by the return value of getenv() may be statically allocated, and can be modified by a subsequent call to getenv(), putenv(3), setenv(3), or unsetenv(3).
Am I misinterpreting these statements?
Thanks.
David
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Duy Nguyen @ 2013-01-05 2:45 UTC (permalink / raw)
To: David Michael; +Cc: git@vger.kernel.org, Junio C Hamano
In-Reply-To: <CAEvUa7niTJVfp8_kuWs50kvhfZ59F-yAuAmeOXEduHXOq-tRFA@mail.gmail.com>
On Sat, Jan 5, 2013 at 7:35 AM, David Michael <fedora.dm0@gmail.com> wrote:
> - if (gitdirenv)
> - return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + if (gitdirenv) {
> + gitdirenv = xstrdup(gitdirenv);
> + ret = setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
> + free(gitdirenv);
> + return ret;
> + }
Maybe we could all this into a wrapper? If getenv() here has a
problem, many other places may have the same problem too. This
simplifies the change. But one has to check that getenv() must not be
used in threaded code.
char *git_getenv(const char *env)
{
static int bufno;
static char *buf[4];
bufno = (bufno + 1) % 4;
free(buf[bufno]);
buf[bufno] = xstrdup(getenv(env));
return buf[bufno];
}
#define getenv(x) git_getenv(x)
--
Duy
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 4:32 UTC (permalink / raw)
To: David Michael; +Cc: git@vger.kernel.org
In-Reply-To: <7vsj6gqvhc.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ... So even if the standard allowed the
> returned value to be volatile across calls to getenv(3),...
> ...
> In fact,
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/getenv.html
>
> says that only ...
Apparently I wasn't even reading what I was quoting carefully
enough. The above does include getenv() as one of the functions
that are allowed to invalidate earlier return values.
Sorry about that. I'll go back to bed (I am a bit under the weather
and OOO today). The conclusion in my original message is still
valid.
> Having said that, we do have codepaths to update a handful of
> environment variables ourselves (GIT_DIR is among them), so I think
> your patch is a good safety measure in general.
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 4:38 UTC (permalink / raw)
To: Duy Nguyen; +Cc: David Michael, git@vger.kernel.org
In-Reply-To: <CACsJy8BeuV8esGTWsQiT_G9pZE28s5KJxH6+dzdhioLgmSiNVg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> Maybe we could all this into a wrapper? If getenv() here has a
> problem, many other places may have the same problem too. This
> simplifies the change. But one has to check that getenv() must not be
> used in threaded code.
That needs to be done regardless, if we care; POSIX explicitly says
getenv() need not be thread-safe.
I personally do not think a wrapper with limited slots is a healthy
direction to go. Most places we use getenv() do not let the return
value live across their scope, and those that do should explicitly
copy the value away. It's between validating that there is _no_ *env()
calls in the codepath between a getenv() call and the use of its
return value, and validating that there is at most 4 such calls there.
The former is much easier to verify and maintain, I think.
^ permalink raw reply
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Duy Nguyen @ 2013-01-05 6:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: David Michael, git@vger.kernel.org
In-Reply-To: <7vk3rsqm6u.fsf@alter.siamese.dyndns.org>
On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
> I personally do not think a wrapper with limited slots is a healthy
> direction to go. Most places we use getenv() do not let the return
> value live across their scope, and those that do should explicitly
> copy the value away. It's between validating that there is _no_ *env()
> calls in the codepath between a getenv() call and the use of its
> return value, and validating that there is at most 4 such calls there.
> The former is much easier to verify and maintain, I think.
I did not look carefully and was scared of 143 getenv calls. But with
about 4 calls, yes it's best to do without the wrapper.
--
Duy
^ permalink raw reply
* Re: [PATCH v4] git-completion.bash: add support for path completion
From: Junio C Hamano @ 2013-01-05 6:27 UTC (permalink / raw)
To: Marc Khouzam, Manlio Perillo; +Cc: git, szeder, felipe.contreras
In-Reply-To: <7vobh4sffw.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>
>> I've been playing with it but I'm not getting the expected
>> behavior when I cd to a sub-directory.
>
> Thanks for testing. Manlio?
Can you try the attached patch?
As I am not familiar with the completion machinery, take this with a
large grain of salt. Here is my explanation of what is going on in
this "how about this" fixup:
* Giving --git-dir from the command line (or GIT_DIR environment)
without specifying GIT_WORK_TREE is to signal Git that you are at
the top of the working tree. "git ls-files" will then show the
full tree even outside the real $cwd because you are lying to
Git.
* "git diff-index" could be told to show only the $cwd and its
subdirectory with the "--relative" option, but it alone is not
sufficient if you throw --git-dir at it; again, you end up lying
that you are at the top.
* As far as I can tell, there is no reason you would want to pass
"--git-dir" to these invocations of ls-files and diff-index. If
the previous call to "__gitdir" could figure out where it is, the
command should be able to figure it out the same way.
There seem to be millions of other existing "git --git-dir=$there"
in this script. As I already said, I am not familiar with the
completion machinery, and I do not know what they are for in the
first place. Perhaps people put them there for a reason, but I do
not know what that reason is.
I think the ones for "for-each-ref", "config" and "stash" should be
harmless. They are commands that do not care about the working
tree.
There is one given to "ls-tree" used in __git_complete_revlist_file;
I do not know if this was intended, what it is for, and if that is
working as intended, though.
I've been CC'ing two people who touched this script rather heavily,
are expected to know the completion machinery, and should be able to
help polishing this topic and moving it forward. Perhaps one of
them can shed light on this.
-- >8 --
Subject: completion: do not pass harmful "--git-dir=$there"
The recently added __git_index_files and __git_diff_index_files
helper functions invoke "ls-files" and "diff_index" while explicitly
passing "--git-dir=$there", to tell them that the invocation is done
at the top of the working tree, which may not be the case when the
user is in a subdirectory. Remove the harmful use of this option,
Tell "diff-index" to show only the paths in the $cwd and show them
relative to the $cwd by passing "--relative". The "ls-files" does
not need this, as that is already its default mode of operation.
---
contrib/completion/git-completion.bash | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c8c6464..f4bd548 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -267,9 +267,9 @@ __git_index_files ()
if [ -d "$dir" ]; then
# NOTE: $1 is not quoted in order to support multiple options
- git --git-dir="$dir" ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
- __git_index_file_list_filter ${2+"$2"} |
- uniq
+ git ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
+ __git_index_file_list_filter ${2+"$2"} |
+ uniq
fi
}
@@ -284,9 +284,9 @@ __git_diff_index_files ()
local dir="$(__gitdir)"
if [ -d "$dir" ]; then
- git --git-dir="$dir" diff-index --name-only "$1" 2>/dev/null |
- __git_index_file_list_filter ${2+"$2"} |
- uniq
+ git diff-index --name-only --relative "$1" 2>/dev/null |
+ __git_index_file_list_filter ${2+"$2"} |
+ uniq
fi
}
^ permalink raw reply related
* Re: [BUG/PATCH] setup: Copy an environment variable to avoid overwrites
From: Junio C Hamano @ 2013-01-05 6:47 UTC (permalink / raw)
To: Duy Nguyen; +Cc: David Michael, git@vger.kernel.org
In-Reply-To: <CACsJy8CZe=qyzmG_1vdLYp07OvkDAU4wYc8MN3et7WBVmMhJOQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Sat, Jan 5, 2013 at 11:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> I personally do not think a wrapper with limited slots is a healthy
>> direction to go. Most places we use getenv() do not let the return
>> value live across their scope, and those that do should explicitly
>> copy the value away. It's between validating that there is _no_ *env()
>> calls in the codepath between a getenv() call and the use of its
>> return value, and validating that there is at most 4 such calls there.
>> The former is much easier to verify and maintain, I think.
>
> I did not look carefully and was scared of 143 getenv calls. But with
> about 4 calls, yes it's best to do without the wrapper.
Just to make sure you did not misunderstand, the 4 (four) in my
message is not about "4 calls among 143 are unsafe".
It was referring to the number of rotating slots your patch defined,
which means
val = getenv("FOO");
... random other code ...
use(val)
is safe only if random other code makes less than 4 getenv() calls.
I didn't verify all of the call sites. It needs to be done with or
without your wrapper patch. Without your wrapper, the validation
needs to make sure "random other code" above does not make any getenv()
call. With your wrapper, the validation needs to make sure "random
other code" above does not make more than 4 such calls. My point
was that the effort needed for both are about the same, so your
wrapper does not buy us much.
^ permalink raw reply
* [PATCH 00/10] push: switch default from "matching" to "simple"
From: Junio C Hamano @ 2013-01-05 6:52 UTC (permalink / raw)
To: git
We promised to change the behaviour of lazy "git push [there]" that
does not say what to push on the command line from "matching" to
"simple" in Git 2.0. Even though the top-level RelNotes symbolic
link points at 1.8.2, we can change our mind to tag 2.0 at the end
of this cycle.
This carries out the threat ;-)
Mmany tests have assumed that the long established default will
never change and relied on the convenience of the "matching"
behaviour. They need to be updated to either specify what they want
from the command line explicitly, or configure the default behaviour
they would expect. And then finally we can flip the default.
Junio C Hamano (10):
t5404: do not assume the "matching" push is the default
t5505: do not assume the "matching" push is the default
t5516: do not assume the "matching" push is the default
t5517: do not assume the "matching" push is the default
t5519: do not assume the "matching" push is the default
t5531: do not assume the "matching" push is the default
t7406: do not assume the "matching" push is the default
t9400: do not assume the "matching" push is the default
t9401: do not assume the "matching" push is the default
push: switch default from "matching" to "simple"
Documentation/config.txt | 6 +++---
builtin/push.c | 31 +++++++------------------------
t/t5404-tracking-branches.sh | 2 +-
t/t5505-remote.sh | 2 +-
t/t5516-fetch-push.sh | 10 +++++-----
t/t5517-push-mirror.sh | 2 +-
t/t5519-push-alternates.sh | 12 ++++++------
t/t5531-deep-submodule-push.sh | 1 +
t/t7406-submodule-update.sh | 4 ++--
t/t9400-git-cvsserver-server.sh | 1 +
t/t9401-git-cvsserver-crlf.sh | 1 +
11 files changed, 29 insertions(+), 43 deletions(-)
--
1.8.1.299.gc73b41f
^ permalink raw reply
* [PATCH 01/10] t5404: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:52 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5404-tracking-branches.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5404-tracking-branches.sh b/t/t5404-tracking-branches.sh
index c240035..2b8c0ba 100755
--- a/t/t5404-tracking-branches.sh
+++ b/t/t5404-tracking-branches.sh
@@ -36,7 +36,7 @@ test_expect_success 'prepare pushable branches' '
'
test_expect_success 'mixed-success push returns error' '
- test_must_fail git push
+ test_must_fail git push origin :
'
test_expect_success 'check tracking branches updated correctly after push' '
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 02/10] t5505: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5505-remote.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index ccc55eb..6579a86 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -345,7 +345,7 @@ test_expect_success 'fetch mirrors do not act as mirrors during push' '
) &&
(cd mirror-fetch/child &&
git branch -m renamed renamed2 &&
- git push parent
+ git push parent :
) &&
(cd mirror-fetch/parent &&
git rev-parse --verify renamed &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 04/10] t5517: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5517-push-mirror.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index e2ad260..12a5dfb 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -256,7 +256,7 @@ test_expect_success 'remote.foo.mirror=no has no effect' '
git branch keep master &&
git push --mirror up &&
git branch -D keep &&
- git push up
+ git push up :
) &&
(
cd mirror &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 03/10] t5516: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5516-fetch-push.sh | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..1a8753d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -247,7 +247,7 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
test_expect_success 'push with matching heads' '
mk_test heads/master &&
- git push testrepo &&
+ git push testrepo : &&
check_push_result $the_commit heads/master
'
@@ -276,7 +276,7 @@ test_expect_success 'push --force with matching heads' '
mk_test heads/master &&
git push testrepo : &&
git commit --amend -massaged &&
- git push --force testrepo &&
+ git push --force testrepo : &&
! check_push_result $the_commit heads/master &&
git reset --hard $the_commit
@@ -507,7 +507,7 @@ test_expect_success 'push with config remote.*.pushurl' '
git checkout master &&
git config remote.there.url test2repo &&
git config remote.there.pushurl testrepo &&
- git push there &&
+ git push there : &&
check_push_result $the_commit heads/master
'
@@ -521,7 +521,7 @@ test_expect_success 'push with dry-run' '
cd testrepo &&
old_commit=$(git show-ref -s --verify refs/heads/master)
) &&
- git push --dry-run testrepo &&
+ git push --dry-run testrepo : &&
check_push_result $old_commit heads/master
'
@@ -981,7 +981,7 @@ test_expect_success 'push --porcelain --dry-run rejected' '
test_expect_success 'push --prune' '
mk_test heads/master heads/second heads/foo heads/bar &&
- git push --prune testrepo &&
+ git push --prune testrepo : &&
check_push_result $the_commit heads/master &&
check_push_result $the_first_commit heads/second &&
! check_push_result $the_first_commit heads/foo heads/bar
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 05/10] t5519: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5519-push-alternates.sh | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/t/t5519-push-alternates.sh b/t/t5519-push-alternates.sh
index c00c9b0..11fcd37 100755
--- a/t/t5519-push-alternates.sh
+++ b/t/t5519-push-alternates.sh
@@ -40,7 +40,7 @@ test_expect_success 'alice works and pushes' '
cd alice-work &&
echo more >file &&
git commit -a -m second &&
- git push ../alice-pub
+ git push ../alice-pub :
)
'
@@ -57,7 +57,7 @@ test_expect_success 'bob fetches from alice, works and pushes' '
git pull ../alice-pub master &&
echo more bob >file &&
git commit -a -m third &&
- git push ../bob-pub
+ git push ../bob-pub :
) &&
# Check that the second commit by Alice is not sent
@@ -86,7 +86,7 @@ test_expect_success 'alice works and pushes again' '
cd alice-work &&
echo more alice >file &&
git commit -a -m fourth &&
- git push ../alice-pub
+ git push ../alice-pub :
)
'
@@ -99,7 +99,7 @@ test_expect_success 'bob works and pushes' '
cd bob-work &&
echo yet more bob >file &&
git commit -a -m fifth &&
- git push ../bob-pub
+ git push ../bob-pub :
)
'
@@ -115,7 +115,7 @@ test_expect_success 'alice works and pushes yet again' '
git commit -a -m sixth.2 &&
echo more and more alice >>file &&
git commit -a -m sixth.3 &&
- git push ../alice-pub
+ git push ../alice-pub :
)
'
@@ -136,7 +136,7 @@ test_expect_success 'bob works and pushes again' '
git hash-object -t commit -w commit &&
echo even more bob >file &&
git commit -a -m seventh &&
- git push ../bob-pub
+ git push ../bob-pub :
)
'
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 06/10] t5531: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t5531-deep-submodule-push.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
index 1947c28..8c16e04 100755
--- a/t/t5531-deep-submodule-push.sh
+++ b/t/t5531-deep-submodule-push.sh
@@ -16,6 +16,7 @@ test_expect_success setup '
(
cd gar/bage &&
git init &&
+ git config push.default matching &&
>junk &&
git add junk &&
git commit -m "Initial junk"
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 07/10] t7406: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t7406-submodule-update.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index feaec6c..c675ce6 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -565,14 +565,14 @@ test_expect_success 'submodule add places git-dir in superprojects git-dir recur
git log > ../../../expected
) &&
git commit -m "added subsubmodule" &&
- git push
+ git push origin :
) &&
(cd .git/modules/deeper/submodule/modules/subsubmodule &&
git log > ../../../../../actual
) &&
git add deeper/submodule &&
git commit -m "update submodule" &&
- git push &&
+ git push origin : &&
test_cmp actual expected
)
'
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 09/10] t9401: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9401-git-cvsserver-crlf.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh
index 1c5bc84..8c3db76 100755
--- a/t/t9401-git-cvsserver-crlf.sh
+++ b/t/t9401-git-cvsserver-crlf.sh
@@ -84,6 +84,7 @@ export CVSROOT CVS_SERVER
rm -rf "$CVSWORK" "$SERVERDIR"
test_expect_success 'setup' '
+ git config push.default matching &&
echo "Simple text file" >textfile.c &&
echo "File with embedded NUL: Q <- there" | q_to_nul > binfile.bin &&
mkdir subdir &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 10/10] push: switch default from "matching" to "simple"
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
We promised to change the behaviour of lazy "git push [there]" that
does not say what to push on the command line from "matching" to
"simple" in Git 2.0.
This finally flips that bit.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 6 +++---
builtin/push.c | 31 +++++++------------------------
2 files changed, 10 insertions(+), 27 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bf8f911..770eefe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1750,15 +1750,15 @@ push.default::
since locally stalled branches will attempt a non-fast forward push
if other users updated the branch.
+
- This is currently the default, but Git 2.0 will change the default
- to `simple`.
+ This used to be the default, and stale web sites may still say so,
+ but Git 2.0 has changed the default to `simple`.
* `upstream` - push the current branch to its upstream branch.
With this, `git push` will update the same remote ref as the one which
is merged by `git pull`, making `push` and `pull` symmetrical.
See "branch.<name>.merge" for how to configure the upstream branch.
* `simple` - like `upstream`, but refuses to push if the upstream
branch's name is different from the local one. This is the safest
- option and is well-suited for beginners. It will become the default
+ option and is well-suited for beginners. It has become the default
in Git 2.0.
* `current` - push the current branch to a branch of the same name.
--
diff --git a/builtin/push.c b/builtin/push.c
index db9ba30..9f7c252 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -24,7 +24,6 @@ static int progress = -1;
static const char **refspec;
static int refspec_nr;
static int refspec_alloc;
-static int default_matching_used;
static void add_refspec(const char *ref)
{
@@ -148,9 +147,9 @@ static void setup_push_upstream(struct remote *remote, int simple)
}
static char warn_unspecified_push_default_msg[] =
-N_("push.default is unset; its implicit value is changing in\n"
+N_("push.default is unset; its implicit value has changed in\n"
"Git 2.0 from 'matching' to 'simple'. To squelch this message\n"
- "and maintain the current behavior after the default changes, use:\n"
+ "and maintain the traditional behavior, use:\n"
"\n"
" git config --global push.default matching\n"
"\n"
@@ -175,14 +174,14 @@ static void setup_default_push_refspecs(struct remote *remote)
{
switch (push_default) {
default:
- case PUSH_DEFAULT_UNSPECIFIED:
- default_matching_used = 1;
- warn_unspecified_push_default_configuration();
- /* fallthru */
case PUSH_DEFAULT_MATCHING:
add_refspec(":");
break;
+ case PUSH_DEFAULT_UNSPECIFIED:
+ warn_unspecified_push_default_configuration();
+ /* fallthru */
+
case PUSH_DEFAULT_SIMPLE:
setup_push_upstream(remote, 1);
break;
@@ -208,12 +207,6 @@ static const char message_advice_pull_before_push[] =
"before pushing again.\n"
"See the 'Note about fast-forwards' in 'git push --help' for details.");
-static const char message_advice_use_upstream[] =
- N_("Updates were rejected because a pushed branch tip is behind its remote\n"
- "counterpart. If you did not intend to push that branch, you may want to\n"
- "specify branches to push or set the 'push.default' configuration variable\n"
- "to 'simple', 'current' or 'upstream' to push only the current branch.");
-
static const char message_advice_checkout_pull_push[] =
N_("Updates were rejected because a pushed branch tip is behind its remote\n"
"counterpart. Check out this branch and merge the remote changes\n"
@@ -227,13 +220,6 @@ static void advise_pull_before_push(void)
advise(_(message_advice_pull_before_push));
}
-static void advise_use_upstream(void)
-{
- if (!advice_push_non_ff_default || !advice_push_nonfastforward)
- return;
- advise(_(message_advice_use_upstream));
-}
-
static void advise_checkout_pull_push(void)
{
if (!advice_push_non_ff_matching || !advice_push_nonfastforward)
@@ -272,10 +258,7 @@ static int push_with_options(struct transport *transport, int flags)
advise_pull_before_push();
break;
case NON_FF_OTHER:
- if (default_matching_used)
- advise_use_upstream();
- else
- advise_checkout_pull_push();
+ advise_checkout_pull_push();
break;
}
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* [PATCH 08/10] t9400: do not assume the "matching" push is the default
From: Junio C Hamano @ 2013-01-05 6:53 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t9400-git-cvsserver-server.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 9502f24..0431386 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -36,6 +36,7 @@ export CVSROOT CVS_SERVER
rm -rf "$CVSWORK" "$SERVERDIR"
test_expect_success 'setup' '
+ git config push.default matching &&
echo >empty &&
git add empty &&
git commit -q -m "First Commit" &&
--
1.8.1.299.gc73b41f
^ permalink raw reply related
* Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes
From: Junio C Hamano @ 2013-01-05 7:54 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list
In-Reply-To: <7v1ue0veww.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Adam Spiers <git@adamspiers.org> writes:
>
>> diff --git a/builtin/clean.c b/builtin/clean.c
>> index 0c7b3d0..bd18b88 100644
>> --- a/builtin/clean.c
>> +++ b/builtin/clean.c
>> @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>> if (!ignored)
>> setup_standard_excludes(&dir);
>>
>> + add_exclude_list(&dir, EXC_CMDL);
>> for (i = 0; i < exclude_list.nr; i++)
>> add_exclude(exclude_list.items[i].string, "", 0,
>> - &dir.exclude_list[EXC_CMDL]);
>> + &dir.exclude_list_groups[EXC_CMDL].ary[0]);
>
> This looks somewhat ugly for two reasons.
>
> * The abstraction add_exclude() offers to its callers is just to
> let them add one pattern to the list of patterns for the kind
> (here, EXC_CMDL); why should they care about .ary[0] part? Are
> there cases any sane caller (not the implementation of the
> exclude_list_group machinery e.g. add_excludes_from_... function)
> may want to call it with .ary[1]? I do not think of any.
> Shouldn't the public API function add_exclude() take a pointer to
> the list group itself?
>
> * When naming an array of things, we tend to prefer naming it
>
> type thing[count]
>
> so that the second element can be called "thing[2]" and not
> "things[2]". dir.exclude_list_group[EXC_CMDL] reads better.
Also, "ary[]" is a bad name, even as an implementation detail, for
two reasons: it is naming it after its type (being an "array") not
after what it is (if it holds the patterns from the same information
source, e.g. file, togeter, "src" might be a better name), and it
uses rather unusual abbreviation (I haven't seen "array" shortened
to "ary" anywhere else).
>
>> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
>> index ef7f99a..c448e06 100644
>> --- a/builtin/ls-files.c
>> +++ b/builtin/ls-files.c
>> @@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,
>> static int option_parse_exclude(const struct option *opt,
>> const char *arg, int unset)
>> {
>> - struct exclude_list *list = opt->value;
>> + struct exclude_list_group *group = opt->value;
>>
>> exc_given = 1;
>> - add_exclude(arg, "", 0, list);
>> + add_exclude(arg, "", 0, &group->ary[0]);
>
> This is another example where the caller would wish to be able to say
>
> add_exclude(arg, "", 0, group);
>
> instead.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox