* Re: [PATCH 2/2] gitk: Fix the geometry when restoring from zoomed state
From: Alexey Borzenkov @ 2009-09-17 13:04 UTC (permalink / raw)
To: Paul Mackerras; +Cc: Pat Thoyts, git
In-Reply-To: <19122.10359.725107.949551@cargo.ozlabs.ibm.com>
On Thu, Sep 17, 2009 at 4:15 PM, Paul Mackerras <paulus@samba.org> wrote:
>> The patch to handle the geometry of a restored gitk by Alexy Borzenkov
>> causes the position of the columns to creep each time the application
>> is restarted. This patch addresses this by remembering the application
>> geometry for the normal state and saving that regardless of the actual
>> state when the application is closed.
>
> So this patch replaces Alexey's patch, then? The context in your patch
> doesn't match the changes made in Alexey's patch AFAICS.
Yes. Pat's patch replaces my patch.
^ permalink raw reply
* Re: Usability question
From: Matthieu Moy @ 2009-09-17 13:09 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Rob Barrett, git
In-Reply-To: <20090917121328.GA21837@neumann>
SZEDER Gábor <szeder@ira.uka.de> writes:
> I tend to aggree, but what about 'git rebase --abort' vs. 'git rebase
> --continue'? IMHO they are also doing something totally different.
If I were to rewrite it, I'd call them "git rebase abort" without
dashes. Not sure renaming them to subcommands is worth it though.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v3 3/4] reset: add option "--merge-safe" to "git reset"
From: Johannes Sixt @ 2009-09-17 13:05 UTC (permalink / raw)
To: Christian Couder
Cc: Junio C Hamano, Christian Couder, git, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Linus Torvalds
In-Reply-To: <c07716ae0909170512k3c2ec7m21e69e9acfdfd81f@mail.gmail.com>
Christian Couder schrieb:
> On Thu, Sep 17, 2009 at 9:24 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Junio C Hamano schrieb:
>>> Johannes Sixt <j.sixt@viscovery.net> writes:
>> $ git stash
>> $ git reset --hard $target
>> $ git stash pop
>>
>>> I have _no_ idea what the intended use-case of --merge-safe is, and that
>>> was why I asked Christian for clarification in the previous round. The
>>> answer was still not clear enough so I pointed out --merge-safe could be
>>> still doing a wrong thing even in _his_ use-case.
>> Reading Christian in 200909170554.49416.chriscool@tuxfamily.org, I think
>> this *is* his use-case? Christian?
>
> Yes, I agree, it can be used instead of git stash.
It "can"? Do you say that you intend --merge-safe for something else in
addition to the above stash + reset --hard + stash pop sequence? What?
> By the way Linus, in his
> patch that added the --merge option, said that --merge could be used like
> that.
But that use-case has one important difference: You can't use stash right
before the reset:
# work tree is dirty
$ git pull $there $topic # assume we have conflicts
# investigate result ...
# oh no, that's crap, scratch it
$ git stash what? conflicted changes?
$ git reset what? --hard would remove my dirty state, too
You are screwed. 'git reset --merge' comes to rescue.
I'm pretty sure you don't mean --merge-safe to provide extra safety in
*this* use-case, but that you have a very different use-case in mind.
-- Hannes
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Alexey Borzenkov @ 2009-09-17 13:02 UTC (permalink / raw)
To: Johannes Sixt
Cc: Marius Storm-Olsen, git, Johannes.Schindelin, msysgit, gitster,
j6t, lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1FB79.5070903@viscovery.net>
This is slightly more than a repost, forgot to reply to all, then
found more info.
On Thu, Sep 17, 2009 at 1:03 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> First thing to do is to understand what is going on: There are other
> architectures that do not have nsec and that do *not* have the problem;
> why do we have a problem on Windows? If you cannot answer this question,
> an nsec solution would still just be "it happens to work", and not "it
> works by design".
Ah, yes, I didn't realize that nsec checking isn't even enabled by
default. So implementing it wouldn't help. As for st_ino it doesn't
matter in this case, as it wouldn't change anyway.
It's interesting that putting git diff or git status before calling
git stash shows that the changes are actually there (and then they can
be picked up). It seems that something manages to spend enough time so
that cache_entry's mtime and index.timestamp start to differ. For
example, here I modified check-racy to show more info for this
particular case:
diff --git a/Makefile b/Makefile
index e0f9a63..00f425c 100644
--- a/Makefile
+++ b/Makefile
@@ -364,6 +364,7 @@ PROGRAMS += git-unpack-file$X
PROGRAMS += git-update-server-info$X
PROGRAMS += git-upload-pack$X
PROGRAMS += git-var$X
+PROGRAMS += git-check-racy$X
# List built-in command $C whose implementation cmd_$C() is not in
# builtin-$C.o but is linked in as part of some other command.
diff --git a/check-racy.c b/check-racy.c
index 00d92a1..72b2878 100644
--- a/check-racy.c
+++ b/check-racy.c
@@ -16,6 +16,12 @@ int main(int ac, char **av)
continue;
}
+ if (!strcmp(ce->name, "file")) {
+ printf("for file: index.timestamp.sec: %d\n",
the_index.timestamp.sec);
+ printf("for file: ce->ce_mtime.sec: %d\n",
ce->ce_mtime.sec);
+ printf("for file: st.st_mtime: %d\n",
(int)st.st_mtime);
+ }
+
if (ce_match_stat(ce, &st, 0))
dirty++;
else if (ce_match_stat(ce, &st, CE_MATCH_RACY_IS_DIRTY))
diff --git a/git-stash.sh b/git-stash.sh
index 03e589f..6eb73fe 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -74,8 +74,14 @@ create_stash () {
cp -p ${GIT_INDEX_FILE-"$GIT_DIR/index"} "$TMP-index" &&
GIT_INDEX_FILE="$TMP-index" &&
export GIT_INDEX_FILE &&
+ echo ">>> before git read-tree:" >&2 &&
+ git check-racy >&2 &&
git read-tree -m $i_tree &&
+ echo ">>> before git add:" >&2 &&
+ git check-racy >&2 &&
git add -u &&
+ echo ">>> before git write-tree" >&2 &&
+ git check-racy >&2 &&
git write-tree &&
rm -f "$TMP-index"
) ) ||
Now where I run it and it doesn't fail:
HEAD is now at dfc6a3a other-file
>>> before git read-tree:
for file: index.timestamp.sec: 1253191568
for file: ce->ce_mtime.sec: 1253191568
for file: st.st_mtime: 1253191568
dirty 1, clean 1, racy 0
>>> before git add:
for file: index.timestamp.sec: 1253191570
for file: ce->ce_mtime.sec: 1253191568
for file: st.st_mtime: 1253191568
dirty 1, clean 1, racy 0
>>> before git write-tree
for file: index.timestamp.sec: 1253191571
for file: ce->ce_mtime.sec: 1253191568
for file: st.st_mtime: 1253191568
dirty 0, clean 2, racy 0
Saved working directory and index state WIP on master: dfc6a3a other-file
HEAD is now at dfc6a3a other-file
(To restore them type "git stash apply")
>>> before git read-tree:
for file: index.timestamp.sec: 1253191572
for file: ce->ce_mtime.sec: 1253191571
for file: st.st_mtime: 1253191572
dirty 1, clean 1, racy 0
>>> before git add:
for file: index.timestamp.sec: 1253191574
for file: ce->ce_mtime.sec: 1253191571
for file: st.st_mtime: 1253191572
dirty 1, clean 1, racy 0
>>> before git write-tree
for file: index.timestamp.sec: 1253191574
for file: ce->ce_mtime.sec: 1253191572
for file: st.st_mtime: 1253191572
dirty 0, clean 2, racy 0
Saved working directory and index state WIP on master: dfc6a3a other-file
But, when this test fails, the results are:
HEAD is now at dfc6a3a other-file
>>> before git read-tree:
for file: index.timestamp.sec: 1253191677
for file: ce->ce_mtime.sec: 1253191676
for file: st.st_mtime: 1253191676
dirty 0, clean 2, racy 0
>>> before git add:
for file: index.timestamp.sec: 1253191679
for file: ce->ce_mtime.sec: 1253191676
for file: st.st_mtime: 1253191676
dirty 0, clean 2, racy 0
>>> before git write-tree
for file: index.timestamp.sec: 1253191679
for file: ce->ce_mtime.sec: 1253191676
for file: st.st_mtime: 1253191676
dirty 0, clean 2, racy 0
Saved working directory and index state WIP on master: dfc6a3a other-file
HEAD is now at dfc6a3a other-file
(To restore them type "git stash apply")
>>> before git read-tree:
for file: index.timestamp.sec: 1253191681
for file: ce->ce_mtime.sec: 1253191680
for file: st.st_mtime: 1253191680
dirty 0, clean 2, racy 0
>>> before git add:
for file: index.timestamp.sec: 1253191683
for file: ce->ce_mtime.sec: 1253191680
for file: st.st_mtime: 1253191680
dirty 0, clean 2, racy 0
>>> before git write-tree
for file: index.timestamp.sec: 1253191683
for file: ce->ce_mtime.sec: 1253191680
for file: st.st_mtime: 1253191680
dirty 0, clean 2, racy 0
Saved working directory and index state WIP on master: dfc6a3a other-file
As you can see, even for "8" (first git stash in that test) it failed:
it seemed as if working tree is clean, index was not updated, changes
were not stashed. But it just so happens that it didn't matter.
Also, as you can see, in both cases index timestamp is slightly bigger
than cache_entry's timestamp. That's why git doesn't see file as racy.
Now I'm wondering WHAT manages to modify index timestamp. :-/
Update:
Ah, I just added more git check-racy calls and found one extremely
interesting case:
HEAD is now at dfc6a3a other-file
>>> start of create_stash:
for file: index.timestamp.sec: 1253192315
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 1, clean 1, racy 0
>>> after update-index -q --refresh:
for file: index.timestamp.sec: 1253192315
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 1, clean 1, racy 0
>>> after checking for changes:
for file: index.timestamp.sec: 1253192315
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 1, clean 1, racy 0
>>> after finding i_tree and i_commit:
for file: index.timestamp.sec: 1253192315
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 1, clean 1, racy 0
>>> before git read-tree:
for file: index.timestamp.sec: 1253192316
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 0, clean 2, racy 0
>>> before git add:
for file: index.timestamp.sec: 1253192318
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 0, clean 2, racy 0
>>> before git write-tree
for file: index.timestamp.sec: 1253192318
for file: ce->ce_mtime.sec: 1253192315
for file: st.st_mtime: 1253192315
dirty 0, clean 2, racy 0
Saved working directory and index state WIP on master: dfc6a3a other-file
"after finding i_tree and i_commit" is immediately before calculating
w_tree. As you can see, "before git read-tree" is off by a second. I
think it's just a bug in msys, cp -p doesn't preserve mtime exactly.
:-/
^ permalink raw reply related
* Re: git workflow for fully distributed mini-teams
From: Rustom Mody @ 2009-09-17 12:38 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <4AB1E514.9030501@viscovery.net>
On Thu, Sep 17, 2009 at 12:58 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> I think the most important thing would be that you send bundles around,
> not patches, so that you all can work with and talk about unique object names.
>
> -- Hannes
Thats what we need -- thanks a 'bundle'!
^ permalink raw reply
* Re: Usability question
From: SZEDER Gábor @ 2009-09-17 12:13 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Rob Barrett, git
In-Reply-To: <vpqy6odhn0d.fsf@bauges.imag.fr>
Hi,
On Thu, Sep 17, 2009 at 12:41:06PM +0200, Matthieu Moy wrote:
> Rob Barrett <barrettboy@gmail.com> writes:
> > 1. What is the distinction that makes the 10% special enough to get
> > non-prefixed options?
>
> Prefixed and non-prefixed is what people usually call respectively
> "options" and "subcommands". To me, the distinction is needed:
>
> Options are flags that modify the behavior of a git command. For
> example, "git reset" and "git reset --hard" do something similar, but
> "git svn rebase" and "git svn dcommit" do something really, totally
> different. It's not about doing the same thing in a different way,
> it's really about different actions.
I tend to aggree, but what about 'git rebase --abort' vs. 'git rebase
--continue'? IMHO they are also doing something totally different.
Best,
Gábor
^ permalink raw reply
* Re: [PATCH v3 3/4] reset: add option "--merge-safe" to "git reset"
From: Christian Couder @ 2009-09-17 12:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Christian Couder, git, Johannes Schindelin, Stephan Beyer,
Daniel Barkalow, Jakub Narebski, Linus Torvalds
In-Reply-To: <7vk4zykv7o.fsf@alter.siamese.dyndns.org>
On Thu, Sep 17, 2009 at 7:15 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <chriscool@tuxfamily.org> writes:
>
>> From: Stephan Beyer <s-beyer@gmx.net>
>>
>> This option is nearly like "--merge" except that it is
>> safer.
>
> Do you still want to have this description after the last round?
>
>> The table below shows what happens when running
>> "git reset --option target" to reset the HEAD to another
>> commit (as a special case "target" could be the same as
>> HEAD) in the cases where "--merge" and "--merge-safe"
>> (abreviated --m-s) behave differently.
>>
>> working index HEAD target working index HEAD
>> B B A A --m-s B A A
>> --merge A A A
>> B B A C --m-s (disallowed)
>> --merge C C C
>
> I'd like to see at least the following rows filled as well.
>
> X U A A --m-s ??? ??? ???
> --merge ??? ??? ???
> X U B A --m-s ??? ??? ???
> --merge ??? ??? ???
>
>> In this table, A, B and C are some different states of a file.
>
> ... and X is "don't care", and U is "unmerged index".
I will have a look, but it seems to me that --m-s and --merge
behave the same in these cases.
>> The code comes from the sequencer GSoC project:
>>
>> git://repo.or.cz/git/sbeyer.git
>>
>> (at commit 5a78908b70ceb5a4ea9fd4b82f07ceba1f019079)
>>
>> But in the sequencer project the "reset" flag was set in the "struct
>> unpack_trees_options" passed to "unpack_trees()". With this flag the
>> changes in the working tree were discarded if the file was different
>> between HEAD and the reset target.
>
> If you need to have four lines worth of description here, is this still
> Stephan's patch, or would it be more appropriate to say "This is based on
> an earlier GSoC work by Stephan in git://repo.or.cz/git/sbeyer.git
> repository." and you take all the credit and blame?
As you insist, I will take all the credit and blame.
>> static const char * const git_reset_usage[] = {
>> - "git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
>> + "git reset [--mixed | --soft | --hard | --merge | --merge-safe] [-q] [<commit>]",
>> "git reset [--mixed] <commit> [--] <paths>...",
>> NULL
>> };
>
> As we established in the previous round, this is _different_ from --merge,
> but *not* in the sense that --merge is more dangerous and users should be
> using this new option instead, but in the sense that --merge perfectly
> works well for its intended use case, and this new option triggers a mode
> of operation that is meant to be used in a completely different use case,
> which is unspecified in this series without documentation.
>
> In that light, is --merge-safe still a good name for the option, or merely
> a misleading one?
I agree that it could be misleading. What about "--stash" ?
> As I said in the previous round, --merge discards the modified index state
> when switching, and it is absolutely _the right thing to do_ in the use
> case it was intended for. It is _not_ dangerous, and using --merge-safe
> in that scenario is not being _safe_ but is actively a _wrong_ thing to do.
Ok.
>> @@ -95,6 +98,16 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
>>
>> read_cache_unmerged();
>>
>> + if (reset_type == MERGE_SAFE) {
>> + unsigned char *head_sha1;
>> + if (get_sha1("HEAD", head_sha1))
>> + return error("You do not have a valid HEAD.");
>> + if (parse_and_init_tree_desc(head_sha1, desc))
>> + return error("Failed to find tree of HEAD.");
>> + nr++;
>> + opts.fn = twoway_merge;
>> + }
>
> get_sha1() takes an allocated buffer, does not allocate space on its own.
> I think you meant "unsigned char head_sha1[20];" here.
Yes.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH 2/2] gitk: Fix the geometry when restoring from zoomed state
From: Paul Mackerras @ 2009-09-17 12:15 UTC (permalink / raw)
To: Pat Thoyts; +Cc: git, Alexy Borzenkov
In-Reply-To: <8763bkcsxb.fsf@users.sourceforge.net>
Pat Thoyts writes:
> The patch to handle the geometry of a restored gitk by Alexy Borzenkov
> causes the position of the columns to creep each time the application
> is restarted. This patch addresses this by remembering the application
> geometry for the normal state and saving that regardless of the actual
> state when the application is closed.
So this patch replaces Alexey's patch, then? The context in your patch
doesn't match the changes made in Alexey's patch AFAICS.
Paul.
^ permalink raw reply
* Re: [PATCH v3 3/4] reset: add option "--merge-safe" to "git reset"
From: Christian Couder @ 2009-09-17 12:12 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Christian Couder, git, Johannes Schindelin,
Stephan Beyer, Daniel Barkalow, Jakub Narebski, Linus Torvalds
In-Reply-To: <4AB1E41F.6050304@viscovery.net>
On Thu, Sep 17, 2009 at 9:24 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Junio C Hamano schrieb:
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>>> Junio C Hamano schrieb:
>>> (2) The intended use-case of --merge-safe is to point the branch head to a
>>> different commit, but to carry the changes that currently are in the index
>>> and wd over to the new commit, similar to checkout --merge.
>
> This is actually an operation that I need quite often, but I can do it
> only by way of git stash.
>
> Clarification: I did not say that I actually meant to carry *only* the
> index and wd changes to the new commit. That is, the operation I have in
> mind can roughly be done in terms of
>
> $ git stash
> $ git reset --hard $target
> $ git stash pop
>
>> I have _no_ idea what the intended use-case of --merge-safe is, and that
>> was why I asked Christian for clarification in the previous round. The
>> answer was still not clear enough so I pointed out --merge-safe could be
>> still doing a wrong thing even in _his_ use-case.
>
> Reading Christian in 200909170554.49416.chriscool@tuxfamily.org, I think
> this *is* his use-case? Christian?
Yes, I agree, it can be used instead of git stash. By the way Linus, in his
patch that added the --merge option, said that --merge could be used like
that.
So for this use case --merge-safe is safer than --merge, and that's why I
suggested that name. But I know I suck at naming things, so there is
probably a better name.
Thanks,
Christian.
^ permalink raw reply
* Re: Usability question
From: Owen Taylor @ 2009-09-17 11:20 UTC (permalink / raw)
To: Rob Barrett; +Cc: git
In-Reply-To: <513ca40e0909170301s2b09184akb27acde76975c09b@mail.gmail.com>
On Thu, 2009-09-17 at 20:01 +1000, Rob Barrett wrote:
> When starting with git people almost always ask some variant of "how
> do I know whether this option should be prefixed with dashes or not?"
> i.e. git reset --hard vs. git stash save --patch, which coupled with
> other path, sha and treeish args make things a bit more confusing.
>
> Not sure if this has been discussed before? If it has point me at the
> discussion and I'll go look at it -- no need to read further.
>
> And people stop asking the question after they get used to git - but
> that's not the same as being usable.
>
> Out of 60+ commands, most take the form
> git <subcommand> [--option]
> and a few take the form
> git <subcommand> subsubcommand [--option]
>
> (a quick scan gives: bisect,bundle,reflog,remote,stash)
>
> My questions:
> 1. What is the distinction that makes the 10% special enough to get
> non-prefixed options?
> 2. Is it worthwhile? Wouldn't it be better if to shoot for more
> consistency / less complexity?
I don't think anybody is going to say that it all makes perfect sense.
One pattern is:
git <verb>
vs.
git <subsystem> <verb> (gui, svn, ...)
git <noun> <verb> (bundle, remote, stash, submodule, ...)
Another pattern is that options don't change the verb, they just
modify it.
But it's easy to find exceptions:
git tag -l
git branch --contains <commit>
git am --abort
I personally think it would help consistency to use the subsubcommand
pattern more and treat 'git tag <tag>' as an shorthand. If you really
want to to create a tag called 'list', you'd need to use
'git tag tag list', or maybe 'git tag -- list'.
Even with compat support for options and a general agreement that I
doubt exists, that's at best a 95% compatible change, so it's unlikely
to happen soon.
- Owen
^ permalink raw reply
* [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Marius Storm-Olsen @ 2009-09-17 11:04 UTC (permalink / raw)
To: git
Cc: Johannes.Schindelin, msysgit, gitster, j6t, lznuaa, raa.lkml,
snaury, Marius Storm-Olsen
In-Reply-To: <4AB212FA.9080102@viscovery.netm>
MinGW set the _CRT_fmode to set both the default fmode and
_O_BINARY on stdin/stdout/stderr. Rather use the main()
define in mingw.h to set this for both MinGW and MSVC.
This will ensure that a MinGW and MSVC build will handle
input and output identically.
Alexey Borzenkov:
Include git-compat-util.h from the test-genrandom.c so
that the application also will use the main() define,
and thus the stdout mode is set correctly and generated
data stays the same.
Johannes Sixt:
Before, since the test-genrandom application would link
against libgit.a, the MinGW process initialization code
would pick up the definition of _CRT_fmode from mingw.c
that is initialized to _O_BINARY. After this patch,
however, the _CRT_fmode symbol is no longer present in
libgit.a, so MinGWs process initialization code will not
set the correct std(in|out|err) mode. So, the include is
needed to override main() and explicitly set the mode.
Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
---
Gaaaaah! Sorry for not reading Alexey's patch properly.
Multitasking is not working to my advantage these days..
compat/mingw.c | 2 --
compat/mingw.h | 5 +++++
test-genrandom.c | 3 +--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 5478b74..5a8fae8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3,8 +3,6 @@
#include <conio.h>
#include "../strbuf.h"
-unsigned int _CRT_fmode = _O_BINARY;
-
static int err_win_to_posix(DWORD winerr)
{
int error = ENOSYS;
diff --git a/compat/mingw.h b/compat/mingw.h
index c43917c..bcd23b0 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -227,12 +227,17 @@ void free_environ(char **env);
/*
* A replacement of main() that ensures that argv[0] has a path
+ * and that default fmode and std(in|out|err) are in binary mode
*/
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(); \
int main(int argc, const char **argv) \
{ \
+ _fmode = _O_BINARY; \
+ _setmode(_fileno(stdin), _O_BINARY); \
+ _setmode(_fileno(stdout), _O_BINARY); \
+ _setmode(_fileno(stderr), _O_BINARY); \
argv[0] = xstrdup(_pgmptr); \
return mingw_main(argc, argv); \
} \
diff --git a/test-genrandom.c b/test-genrandom.c
index 8ad276d..b3c28d9 100644
--- a/test-genrandom.c
+++ b/test-genrandom.c
@@ -4,8 +4,7 @@
* Copyright (C) 2007 by Nicolas Pitre, licensed under the GPL version 2.
*/
-#include <stdio.h>
-#include <stdlib.h>
+#include "git-compat-util.h"
int main(int argc, char *argv[])
{
--
1.6.2.1.418.g33d56.dirty
^ permalink raw reply related
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Johannes Sixt @ 2009-09-17 10:44 UTC (permalink / raw)
To: Marius Storm-Olsen
Cc: git, Johannes.Schindelin, msysgit, gitster, lznuaa, raa.lkml,
snaury
In-Reply-To: <1eeb32498237638df6d954dc65ab5372dd0f04c9.1253174364.git.mstormo@gmail.com>
Marius Storm-Olsen schrieb:
> diff --git a/test-genrandom.c b/test-genrandom.c
> index 8ad276d..5849599 100644
> --- a/test-genrandom.c
> +++ b/test-genrandom.c
> @@ -6,6 +6,7 @@
>
> #include <stdio.h>
> #include <stdlib.h>
> +#include "git-compat-util.h"
>
> int main(int argc, char *argv[])
> {
We usually #include *only* git-compat-util.h; you can remove stdio.h and
stdlib.h, as Alexey proposed.
-- Hannes
^ permalink raw reply
* Re: Usability question
From: Matthieu Moy @ 2009-09-17 10:41 UTC (permalink / raw)
To: Rob Barrett; +Cc: git
In-Reply-To: <513ca40e0909170301s2b09184akb27acde76975c09b@mail.gmail.com>
Rob Barrett <barrettboy@gmail.com> writes:
> My questions:
> 1. What is the distinction that makes the 10% special enough to get
> non-prefixed options?
Prefixed and non-prefixed is what people usually call respectively
"options" and "subcommands". To me, the distinction is needed:
Options are flags that modify the behavior of a git command. For
example, "git reset" and "git reset --hard" do something similar, but
"git svn rebase" and "git svn dcommit" do something really, totally
different. It's not about doing the same thing in a different way,
it's really about different actions.
Subcommands are closer to commands than they are to options. The
reason to group several subcommands into one command is mostly to
reduce the number of commands, but for example, it could have been
decided to replace "git svn dcommit" by "git svn-dcommit" (but then
"git help" would have been really really scarry).
> 2. Is it worthwhile? Wouldn't it be better if to shoot for more
> consistency / less complexity?
Well, if you want to get rid of subcommands, why not get rid of
commands, too?
git --commit
git --status
git --svn --rebase
I find the distinction between commands, subcommands and options
really helpfull.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Usability question
From: Rob Barrett @ 2009-09-17 10:01 UTC (permalink / raw)
To: git
When starting with git people almost always ask some variant of "how
do I know whether this option should be prefixed with dashes or not?"
i.e. git reset --hard vs. git stash save --patch, which coupled with
other path, sha and treeish args make things a bit more confusing.
Not sure if this has been discussed before? If it has point me at the
discussion and I'll go look at it -- no need to read further.
And people stop asking the question after they get used to git - but
that's not the same as being usable.
Out of 60+ commands, most take the form
git <subcommand> [--option]
and a few take the form
git <subcommand> subsubcommand [--option]
(a quick scan gives: bisect,bundle,reflog,remote,stash)
My questions:
1. What is the distinction that makes the 10% special enough to get
non-prefixed options?
2. Is it worthwhile? Wouldn't it be better if to shoot for more
consistency / less complexity?
Rob
^ permalink raw reply
* [RFC/PATCH] branch: die when calling "git branch [-a|-r] branchname".
From: Matthieu Moy @ 2009-09-17 9:35 UTC (permalink / raw)
To: git; +Cc: Matthieu Moy
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
builtin-branch.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 9f57992..fe11b55 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -635,10 +635,13 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
rename_branch(head, argv[0], rename > 1);
else if (rename && (argc == 2))
rename_branch(argv[0], argv[1], rename > 1);
- else if (argc <= 2)
+ else if (argc <= 2) {
+ if (kinds != REF_LOCAL_BRANCH) {
+ die("-a or -r do not make sense with branch name");
+ }
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, track);
- else
+ } else
usage_with_options(builtin_branch_usage, options);
return 0;
--
1.6.5.rc1.11.g2d184.dirty
^ permalink raw reply related
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Marius Storm-Olsen @ 2009-09-17 9:28 UTC (permalink / raw)
To: Johannes Sixt
Cc: Alexey Borzenkov, git, Johannes.Schindelin, msysgit, gitster, j6t,
lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1FB79.5070903@viscovery.net>
Johannes Sixt said the following on 17.09.2009 11:03:
> Alexey Borzenkov schrieb:
>> Do you want me to cook up a patch?
>
> I consider an answer to the question "why do we need nsec?" more important.
>
> BTW: t4130-apply-criss-cross-rename.sh fails quite often as well. This
> time it is because we fill in st_ino = 0 instead of some useful value. The
> test moves files around that happen to have identical sizes. After the
> operation, the dirty working tree could be noticed because st_ino differs
> from the index. But since we always set it to 0, it is not noticed.
> (That's my theory.)
>
> If you make a patch, can you think about this issue as well?
Cygwin version also sets st_ino = 0, so I'm not sure this is the cause
of failure; unless this test also fails there? Does anyone know?
--
.marius
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2009, #04; Wed, 16)
From: Sverre Rabbelier @ 2009-09-17 9:26 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce; +Cc: git
In-Reply-To: <7v1vm6kskd.fsf@alter.siamese.dyndns.org>
Heya,
On Thu, Sep 17, 2009 at 08:12, Junio C Hamano <gitster@pobox.com> wrote:
> * sr/gfi-options (2009-09-06) 6 commits
> (merged to 'next' on 2009-09-07 at 5f6b0ff)
> + fast-import: test the new option command
> + fast-import: add option command
> + fast-import: test the new feature command
> + fast-import: add feature command
> + fast-import: put marks reading in it's own function
> + fast-import: put option parsing code in separate functions
> (this branch is used by jh/notes.)
>
> Ping?
I replied to Shawn's comment by asking how he wants to go from here; I
am waiting for him to find time to reply :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Johannes Sixt @ 2009-09-17 9:03 UTC (permalink / raw)
To: Alexey Borzenkov
Cc: Marius Storm-Olsen, git, Johannes.Schindelin, msysgit, gitster,
j6t, lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <e2480c70909170139x9580bb4pcc1ad8e1b93aef17@mail.gmail.com>
Alexey Borzenkov schrieb:
> On Thu, Sep 17, 2009 at 12:10 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Marius Storm-Olsen schrieb:
>>> BTW, I ran all the tests (make /k test) before and after the
>>> whole series, with msysgit 'devel' branch + plain git.git 'next',
>>> and it turns out that 2 more tests pass after this series :)
>> Sorry to disappoint you: these 2 tests only passed by chance. For a
>> work-around see e95a73ef in mingw.git (it papers over an undetected
>> racily-clean index).
>
> Maybe we should just implement nsec for mingw? The code is already almost there:
First thing to do is to understand what is going on: There are other
architectures that do not have nsec and that do *not* have the problem;
why do we have a problem on Windows? If you cannot answer this question,
an nsec solution would still just be "it happens to work", and not "it
works by design".
I think I had analyzed this particular case, and I understood it, but did
not make notes why I found it satisfactory to just paper over the issue,
and today I don't remember anymore. :-/
> Since we already reimplement stat/lstat/fstat it should be relatively
> simple and we don't care about usage of mingw's stat.
Don't forget utime().
> Do you want me to cook up a patch?
I consider an answer to the question "why do we need nsec?" more important.
BTW: t4130-apply-criss-cross-rename.sh fails quite often as well. This
time it is because we fill in st_ino = 0 instead of some useful value. The
test moves files around that happen to have identical sizes. After the
operation, the dirty working tree could be noticed because st_ino differs
from the index. But since we always set it to 0, it is not noticed.
(That's my theory.)
If you make a patch, can you think about this issue as well?
-- Hannes
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Alexey Borzenkov @ 2009-09-17 8:57 UTC (permalink / raw)
To: Marius Storm-Olsen
Cc: Johannes Sixt, git, Johannes.Schindelin, msysgit, gitster, j6t,
lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1F71B.6050004@gmail.com>
On Thu, Sep 17, 2009 at 12:45 PM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
> we already override the stat() implementation with our own :)
Yes, that's why implementing nsec whould be easy. See compat/cygwin.c
(filetime_to_timespec) and git-compat-util.h (#ifdef NO_NSEC).
Currently mingw doesn't support tv_nsec.
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Marius Storm-Olsen @ 2009-09-17 8:45 UTC (permalink / raw)
To: Alexey Borzenkov
Cc: Johannes Sixt, git, Johannes.Schindelin, msysgit, gitster, j6t,
lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <e2480c70909170139x9580bb4pcc1ad8e1b93aef17@mail.gmail.com>
Alexey Borzenkov said the following on 17.09.2009 10:39:
> On Thu, Sep 17, 2009 at 12:10 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Marius Storm-Olsen schrieb:
>>> BTW, I ran all the tests (make /k test) before and after the
>>> whole series, with msysgit 'devel' branch + plain git.git 'next',
>>> and it turns out that 2 more tests pass after this series :)
>> Sorry to disappoint you: these 2 tests only passed by chance. For a
>> work-around see e95a73ef in mingw.git (it papers over an undetected
>> racily-clean index).
>
> Maybe we should just implement nsec for mingw? The code is already almost there:
>
> Converter filetime_to_timespec is available in cygwin.c
> Definition of struct timespec and struct stat that uses it can be
> borrowed from cygwin's cygwin/stat.h and cygwin/types.h, we could call
> it struct mingw_timespec and struct mingw_stat. Then #define timespec
> mingw_timespec and #define stat mingw_stat, as well as #define
> st_atime (st_atim.tv_sec) and similarly others, just like
> cygwin/stat.h does.
> Since we already reimplement stat/lstat/fstat it should be relatively
> simple and we don't care about usage of mingw's stat.
> Do you want me to cook up a patch?
we already override the stat() implementation with our own :)
compat/mingw.h (~174):
/* Use mingw_lstat() instead of lstat()/stat() and
* mingw_fstat() instead of fstat() on Windows.
*/
#define off_t off64_t
#define stat _stati64
#define lseek _lseeki64
int mingw_lstat(const char *file_name, struct stat *buf);
int mingw_fstat(int fd, struct stat *buf);
#define fstat mingw_fstat
#define lstat mingw_lstat
#define _stati64(x,y) mingw_lstat(x,y)
int mingw_utime(const char *file_name, const struct utimbuf *times);
#define utime mingw_utime
See compat/mingw.c for implementation..
--
.marius
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Alexey Borzenkov @ 2009-09-17 8:39 UTC (permalink / raw)
To: Johannes Sixt
Cc: Marius Storm-Olsen, git, Johannes.Schindelin, msysgit, gitster,
j6t, lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1EF0E.5010600@viscovery.net>
On Thu, Sep 17, 2009 at 12:10 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Marius Storm-Olsen schrieb:
>> BTW, I ran all the tests (make /k test) before and after the
>> whole series, with msysgit 'devel' branch + plain git.git 'next',
>> and it turns out that 2 more tests pass after this series :)
> Sorry to disappoint you: these 2 tests only passed by chance. For a
> work-around see e95a73ef in mingw.git (it papers over an undetected
> racily-clean index).
Maybe we should just implement nsec for mingw? The code is already almost there:
Converter filetime_to_timespec is available in cygwin.c
Definition of struct timespec and struct stat that uses it can be
borrowed from cygwin's cygwin/stat.h and cygwin/types.h, we could call
it struct mingw_timespec and struct mingw_stat. Then #define timespec
mingw_timespec and #define stat mingw_stat, as well as #define
st_atime (st_atim.tv_sec) and similarly others, just like
cygwin/stat.h does.
Since we already reimplement stat/lstat/fstat it should be relatively
simple and we don't care about usage of mingw's stat.
Do you want me to cook up a patch?
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Marius Storm-Olsen @ 2009-09-17 8:14 UTC (permalink / raw)
To: Johannes Sixt
Cc: Alexey Borzenkov, git, Johannes.Schindelin, msysgit, gitster, j6t,
lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1EF0E.5010600@viscovery.net>
Johannes Sixt said the following on 17.09.2009 10:10:
> Marius Storm-Olsen schrieb:
>> BTW, I ran all the tests (make /k test) before and after the
>> whole series, with msysgit 'devel' branch + plain git.git 'next',
>> and it turns out that 2 more tests pass after this series :)
>
> Sorry to disappoint you: these 2 tests only passed by chance. For a
> work-around see e95a73ef in mingw.git (it papers over an undetected
> racily-clean index).
Ah, gotcha. Oh well, at least no regressions then, which is also a
good thing :)
--
.marius
^ permalink raw reply
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Johannes Sixt @ 2009-09-17 8:10 UTC (permalink / raw)
To: Marius Storm-Olsen
Cc: Alexey Borzenkov, git, Johannes.Schindelin, msysgit, gitster, j6t,
lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1EB02.2090308@gmail.com>
Marius Storm-Olsen schrieb:
> BTW, I ran all the tests (make /k test) before and after the
> whole series, with msysgit 'devel' branch + plain git.git 'next',
> and it turns out that 2 more tests pass after this series :)
Sorry to disappoint you: these 2 tests only passed by chance. For a
work-around see e95a73ef in mingw.git (it papers over an undetected
racily-clean index).
-- Hannes
^ permalink raw reply
* [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Marius Storm-Olsen @ 2009-09-17 8:02 UTC (permalink / raw)
To: git
Cc: Johannes.Schindelin, msysgit, gitster, j6t, lznuaa, raa.lkml,
snaury, Marius Storm-Olsen
In-Reply-To: <929c5a34cd2621af24bcda7e47ff2e76b51c2e09.1253088099.git.mstormo@gmail.com>
MinGW set the _CRT_fmode to set both the default fmode and
_O_BINARY on stdin/stdout/stderr. Rather use the main()
define in mingw.h to set this for both MinGW and MSVC.
This will ensure that a MinGW and MSVC build will handle
input and output identically.
Alexey Borzenkov:
Include git-compat-util.h from the test-genrandom.c so
that the application also will use the main() define,
and thus the stdout mode is set correctly and generated
data stays the same.
Johannes Sixt:
Before, since the test-genrandom application would link
against libgit.a, the MinGW process initialization code
would pick up the definition of _CRT_fmode from mingw.c
that is initialized to _O_BINARY. After this patch,
however, the _CRT_fmode symbol is no longer present in
libgit.a, so MinGWs process initialization code will not
set the correct std(in|out|err) mode. So, the include is
needed to override main() and explicitly set the mode.
Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
---
compat/mingw.c | 2 --
compat/mingw.h | 5 +++++
test-genrandom.c | 1 +
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 5478b74..5a8fae8 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -3,8 +3,6 @@
#include <conio.h>
#include "../strbuf.h"
-unsigned int _CRT_fmode = _O_BINARY;
-
static int err_win_to_posix(DWORD winerr)
{
int error = ENOSYS;
diff --git a/compat/mingw.h b/compat/mingw.h
index c43917c..bcd23b0 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -227,12 +227,17 @@ void free_environ(char **env);
/*
* A replacement of main() that ensures that argv[0] has a path
+ * and that default fmode and std(in|out|err) are in binary mode
*/
#define main(c,v) dummy_decl_mingw_main(); \
static int mingw_main(); \
int main(int argc, const char **argv) \
{ \
+ _fmode = _O_BINARY; \
+ _setmode(_fileno(stdin), _O_BINARY); \
+ _setmode(_fileno(stdout), _O_BINARY); \
+ _setmode(_fileno(stderr), _O_BINARY); \
argv[0] = xstrdup(_pgmptr); \
return mingw_main(argc, argv); \
} \
diff --git a/test-genrandom.c b/test-genrandom.c
index 8ad276d..5849599 100644
--- a/test-genrandom.c
+++ b/test-genrandom.c
@@ -6,6 +6,7 @@
#include <stdio.h>
#include <stdlib.h>
+#include "git-compat-util.h"
int main(int argc, char *argv[])
{
--
1.6.2.1.418.g33d56.dirty
^ permalink raw reply related
* Re: [PATCH 04/15] Set _O_BINARY as default fmode for both MinGW and MSVC
From: Marius Storm-Olsen @ 2009-09-17 7:53 UTC (permalink / raw)
To: Johannes Sixt
Cc: Alexey Borzenkov, git, Johannes.Schindelin, msysgit, gitster, j6t,
lznuaa, raa.lkml, Marius Storm-Olsen
In-Reply-To: <4AB1E6E4.1040100@viscovery.net>
[-- Attachment #1: Type: text/plain, Size: 641 bytes --]
Johannes Sixt said the following on 17.09.2009 09:36:
> Marius Storm-Olsen schrieb:
>> Yup, are you ok with squashing this hunk into the patch then?
>
> Of course; with some extra words in the commit message.
Great. Hold tight, I'll resend an updated version.
BTW, I ran all the tests (make /k test) before and after the
whole series, with msysgit 'devel' branch + plain git.git 'next',
and it turns out that 2 more tests pass after this series :)
Before:
fixed 4
success 4017
failed 380
broken 9
total 4524
After:
fixed 4
success 4019
failed 378
broken 9
total 4524
Both complete logs attached in 7zip format.
--
.marius
[-- Attachment #2: testresults.7z --]
[-- Type: application/octet-stream, Size: 44812 bytes --]
^ 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