* Re: [msysGit] [PATCH 07/17] Fix __stdcall/WINAPI placement and function prototype
From: Johannes Schindelin @ 2009-09-15 0:24 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Marius Storm-Olsen, msysgit, git, lznuaa
In-Reply-To: <200909142200.52174.j6t@kdbg.org>
Hi,
On Mon, 14 Sep 2009, Johannes Sixt wrote:
> On Montag, 14. September 2009, Marius Storm-Olsen wrote:
> > WINAPI is a macro which translates into the proper calling convention, so
> > replace __stdcall with that.
>
> I've already pointed out elsewhere that the documentation of
> _beginthreadex explicitly says that the calling convention of the
> function pointer must be __stdcall. It does not mention WINAPI.
> Therefore, I think that these two changes are not correct:
>
> > -static __stdcall unsigned ticktack(void *dummy)
> > +static unsigned WINAPI ticktack(void *dummy)
>
> > -static __stdcall unsigned run_thread(void *data)
> > +static unsigned WINAPI run_thread(void *data)
>
> You should s/WINAPI/__stdcall/.
I don't think that comments by our most proficient MSys guy should be
disregarded as thee comments were.
Ciao,
Dscho
^ permalink raw reply
* Re: Patches for git-push --confirm and --show-subjects
From: Owen Taylor @ 2009-09-14 23:18 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0909141745410.14907@iabervon.org>
On Mon, 2009-09-14 at 18:21 -0400, Daniel Barkalow wrote:
> On Sun, 13 Sep 2009, Owen Taylor wrote:
[...]
> I think the classification logic should move to match_refs(), assuming you
> mean the ref->nonfastforward and ref->deletion stuff. It would probably
> also be worth having a bit for "already up to date". (Note that
> cmd_send_pack() calls match_refs(), so there wouldn't have to be
> duplication between the legacy cmd_send_pack() code path and the
> transport_push() codepath if the code moved into match_refs()).
[...]
> > - You add another vfunc to the transport - '->get_capabilities' or
> > something - that encapsulates server_supports("delete-refs").
>
> I think it would be better to have a vfunc that takes refs with the
> classification bits set and sets the statuses based on the idea that we're
> not going to lose any races and the remote won't reject our change for
> some reason we don't know about. There's a potentially large and varied
> set of restrictions on what the other side is willing to accept, and I
> think it would be better to put that on the other side of the vfunc,
> rather than having the main transport code know that "delete-refs" means
> that you can delete refs, "nonfastforward" means you can force a
> non-fast-forward, something means you can create files named "CVS", etc.
match_refs seems like a reasonable place to put this logic, but I'm not
sure I completely follow what you are proposing in terms of
transport-specific customization.
match_refs() is called from a couple of places where there is no
'transport' (cmd_send_pack() and http-push.c) so it can't itself call
into the transport code.
Are you thinking of a virtual function that would be a "second pass"
after the main logic done is done by match_refs; so ->check_refs()
virtual function?
How would the 'bit for "already up to date"' differ from
REF_STATUS_UPTODATE. ?
> I think a pre-push hook would be popular; I know I'd like to have a hook
> that makes sure that I signed off anything I'm pushing (when the server
> might check that *somebody* did, but wouldn't know that this push is
> supposed to be me), and I'd like a hook that checks that I've referenced
> an issue in an issue tracker for each commit that I'm pushing (but only
> when I go to push it).
If I can figure out the rest of it, I'll look at adding a hook on top as
a sweetener :-)
- Owen
^ permalink raw reply
* Re: could not detach HEAD error didn't identify the cause of the issue
From: Jeff King @ 2009-09-14 22:39 UTC (permalink / raw)
To: Ben Bradshaw; +Cc: git
In-Reply-To: <4AAEAF28.2050604@catalyst.net.nz>
[Please don't top-post; the usual etiquette on this list is to reply
in-line].
On Tue, Sep 15, 2009 at 09:01:28AM +1200, Ben Bradshaw wrote:
> I'm using git version 1.5.6.5 - I think I can elaborate on the issue
> some more though. I'll step through the process that got me to the point
> of failure.
Thanks for following up. I was able to reproduce your problem. The more
complex setup you described is not necessary to reproduce (though thank
you for providing it, as it eliminated one possible source of
variation). It really is just about having an untracked file 'foo' in
your working tree, and the thing you are trying to rebase on top of also
has 'foo' as a tracked file.
The good news is that it has already been fixed. I bisected it down to:
commit 324c2c3177ca5b62b12029ce8821542fc29a2733
Author: Junio C Hamano <gitster@pobox.com>
Date: Mon Jul 14 14:05:35 2008 -0700
git-rebase: report checkout failure
When detaching the HEAD to the base commit, the "git checkout"
command could fail if, for example, upstream contains a file that
would overrwrite a local, untracked file. Unconditionally
discarding the standard error stream was done to squelch the
progress and notices back when checkout did not have -q option,
but there is no reason to keep doing it anymore.
Noticed by Robert Shearman.
which sounds about right. :)
It made it into git v1.6.0, but not into any of the v1.5.x maintenance
series. I would suggest upgrading if you can.
-Peff
^ permalink raw reply
* Re: Patches for git-push --confirm and --show-subjects
From: Daniel Barkalow @ 2009-09-14 22:21 UTC (permalink / raw)
To: Owen Taylor; +Cc: Junio C Hamano, git
In-Reply-To: <1252895719.11581.53.camel@localhost.localdomain>
On Sun, 13 Sep 2009, Owen Taylor wrote:
> On Sun, 2009-09-13 at 17:47 -0700, Junio C Hamano wrote:
> > * I do not think it should use two separate push_refs call into transport
> > (first with dry-run and second with real).
> >
> > Immediately after match_refs() call in transport_push(), you know if
> > the push is a non-fast-forward (in which case you do not know what you
> > will be losing anyway because you haven't seen what you are missing
> > from the other end) or exactly what your fast-forward push will be
> > sending, so between that call and the actual transport->push_refs()
> > would be the ideal place to call the hook, with a list of "ref old
> > new", without running a dry-run.
>
> The reason I had to do two calls to transport->push_refs is not because
> it actually pushes the refs twice. It's because the logic for
> classifying the refs is in builtin-send-pack.c. When you pass in
> args.dry_run=1 you get the classification logic without the network
> traffic.
I think the classification logic should move to match_refs(), assuming you
mean the ref->nonfastforward and ref->deletion stuff. It would probably
also be worth having a bit for "already up to date". (Note that
cmd_send_pack() calls match_refs(), so there wouldn't have to be
duplication between the legacy cmd_send_pack() code path and the
transport_push() codepath if the code moved into match_refs()).
> (There's a little messiness about whether it sends the "flush" 0000 or
> not that I had to work around, but that's peripheral.)
>
> The way to clean it up is pretty obvious:
>
> - You add another vfunc to the transport - '->get_capabilities' or
> something - that encapsulates server_supports("delete-refs").
I think it would be better to have a vfunc that takes refs with the
classification bits set and sets the statuses based on the idea that we're
not going to lose any races and the remote won't reject our change for
some reason we don't know about. There's a potentially large and varied
set of restrictions on what the other side is willing to accept, and I
think it would be better to put that on the other side of the vfunc,
rather than having the main transport code know that "delete-refs" means
that you can delete refs, "nonfastforward" means you can force a
non-fast-forward, something means you can create files named "CVS", etc.
This is pretty similar to having a "dry run" call first, except that it
wouldn't end up rechecking the same things on the real run immediately
following the dry run, because the checking code has moved to a separate
method.
Of course, this step isn't entirely needed; without it, you just get asked
"Are you sure?" for changes the local side can tell won't be permitted, in
addition to for changes the local side can't tell won't be permitted.
(Like, you're allowed to delete refs in general, but not the one you're
trying to delete.)
> - You split the classification logic out into a helper function
> (maybe still in builtin-send-pack.c, maybe moved into some other
> file... don't know what's appropriate.)
>
> After all, if there was another push_refs backend, it shouldn't be
> duplicating the classification logic...
I think match_refs should do it.
> - You pass pre-classified ref updates to ->push_refs
>
> I don't know how that interacts with other planned changes to this code.
I think this is a good thing to clean up before further changes; any other
improvements would either duplicate the logic that's in builtin-send-pack
or be missing it.
> > for a few reasons.
> >
> > (1) When push.confirm is set, you do not want to interact with the user
> > when the standard input is not a terminal. But an automated script
> > that runs git-push can still use an appropriate pre-push hook to make
> > the decision to intervene without human presense.
> >
> > (2) As your --show-subjects patch shows, the likes and dislikes of the
> > output format for confirmation would be highly personal. A separate
> > hook that is fed list of <ref, old, new> would make it easier to
> > customize this to suite people's tastes.
>
> The --show-subjects idea is equally useful for --dry-run. And even when
> for successful/failed pushes when neither --confirm not --dry-run is
> passed.
>
> I'm not that convinced that there's that much scope for configurability
> in this area. Clearly there's some arbitrary decisions I made - that
> abbreviated hashes wouldn't be useful. That up to 8 commit subjects
> should be shown. Etc.
>
> But as yet, there's no data as to whether people would actually want to
> make *different* arbitrary decisions.
>
> Adding more configurability (formats, etc.) doesn't really bother me,
> though it does seem like coding in advance of need. But what would
> bother me is if the feature isn't useful without complex configuration
> or installing custom scripts.
I think a pre-push hook would be popular; I know I'd like to have a hook
that makes sure that I signed off anything I'm pushing (when the server
might check that *somebody* did, but wouldn't know that this push is
supposed to be me), and I'd like a hook that checks that I've referenced
an issue in an issue tracker for each commit that I'm pushing (but only
when I go to push it).
But I think a simple "ask an interactive user" check makes sense to have
in the same part of the code.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: could not detach HEAD error didn't identify the cause of the issue
From: Ben Bradshaw @ 2009-09-14 21:01 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <4AAEAE15.7050607@catalyst.net.nz>
Hi Jeff,
I'm using git version 1.5.6.5 - I think I can elaborate on the issue
some more though. I'll step through the process that got me to the point
of failure. For the purposes of this example 'ben@carbon' is me at my
local machine, 'ben@staging' is our staging server. Our development
setup for this project has us all working on the staging server except
when we need to test something that we don't want on staging (could
break things) so I also have a local copy of staging. (This seems
backwards to me but it's working rather well for the short time line we
have).
In this case I did the following:
1. Add module site_map to carbon
2. Install and test on carbon
3. Add module site_map to staging
4. Install and test on staging
5. git commit && git push on staging
6. git pull --rebase on carbon
I think you will need two checkouts of your repo in order to trigger
this case?
Ben
>
> Jeff King wrote:
>> On Mon, Sep 14, 2009 at 12:14:24PM +1200, Ben Bradshaw wrote:
>>
>>
>>> I was asked to post this issue to the list, I've just had git tell me it
>>> can't detach HEAD which left me scratching mine as to the cause and fix.
>>> Turns out if I have local untracked files that the remote branch also
>>> has then this error will trigger. I have included my shell backlog
>>> (client data removed). In this example the site_map folder was present
>>> on master and I had a local, untracked copy.
>>>
>>> git can obviosly detect this issue and stop my local copy getting in a
>>> bad state, but an error message such as "sites/all/modules/site_map/ is
>>> present in HEAD but is untracked locally, unable to apply changes" (or
>>> something to point the finger) would be much appreciated.
>>>
>>
>> I couldn't reproduce it here; I get such a message.
>>
>> # set up forked repo, "another" exists only on master
>> $ mkdir repo && cd repo
>> $ git init
>> $ echo content >file && git add file && git commit -m base
>> $ echo content >another && git add another && git commit -m another
>> $ git checkout -b other HEAD^
>> $ echo changes >file && git commit -a -m changes
>>
>> # add untracked "another" here
>> $ echo untracked >another
>>
>> # try rebase
>> $ git rebase master
>> First, rewinding head to replay your work on top of it...
>> error: Untracked working tree file 'another' would be overwritten by merge.
>> could not detach HEAD
>>
>> # try git pull --rebase, in case it hides the message
>> $ git config branch.other.remote .
>> $ git config branch.other.merge refs/heads/master
>> $ git pull --rebase
>> From .
>> * branch master -> FETCH_HEAD
>> First, rewinding head to replay your work on top of it...
>> error: Untracked working tree file 'another' would be overwritten by merge.
>> could not detach HEAD
>>
>> I'll admit the "could not detach HEAD" message would probably be better as:
>>
>> rebase: unable to move HEAD to 'master', aborting rebase
>>
>> or something similar.
>>
>> What version of git are you using? What does my test case above produce
>> for you?
>>
>> -Peff
>>
>>
^ permalink raw reply
* Re: [PATCH 07/17] Fix __stdcall/WINAPI placement and function prototype
From: Johannes Sixt @ 2009-09-14 20:00 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: msysgit, Johannes.Schindelin, git, lznuaa
In-Reply-To: <1d699897d4163224ce22db9400569fab20547e9d.1252925290.git.mstormo@gmail.com>
On Montag, 14. September 2009, Marius Storm-Olsen wrote:
> WINAPI is a macro which translates into the proper calling convention, so
> replace __stdcall with that.
I've already pointed out elsewhere that the documentation of _beginthreadex
explicitly says that the calling convention of the function pointer must be
__stdcall. It does not mention WINAPI. Therefore, I think that these two
changes are not correct:
> -static __stdcall unsigned ticktack(void *dummy)
> +static unsigned WINAPI ticktack(void *dummy)
> -static __stdcall unsigned run_thread(void *data)
> +static unsigned WINAPI run_thread(void *data)
You should s/WINAPI/__stdcall/.
-- Hannes
^ permalink raw reply
* Re: [PATCH 14/17] Add ftruncate implementation for MSVC
From: Junio C Hamano @ 2009-09-14 19:48 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <c95a367520435feec0833d2d0edc33d86cf939a8.1252925290.git.mstormo@gmail.com>
Marius Storm-Olsen <mstormo@gmail.com> writes:
> The MSVC Posix implementation doesn't contain ftruncate, so add our own
> which can handle large files (64bit offsets).
>
> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
> ---
> compat/msvc.c | 8 ++++++++
> compat/msvc.h | 2 ++
> 2 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/compat/msvc.c b/compat/msvc.c
> index ac04a4c..b96b045 100644
> --- a/compat/msvc.c
> +++ b/compat/msvc.c
> @@ -32,4 +32,12 @@ int closedir(DIR *dir)
> return 0;
> }
>
> +int ftruncate(int fd, __int64 length)
> +{
> + HANDLE fh = (HANDLE)_get_osfhandle(fd);
> + if (!fh || _lseeki64(fd, length, SEEK_SET))
> + return -1;
> + return SetEndOfFile(fh) ? 0 : -1;
> +}
> +
Wouldn't it break this toy program? ftruncate() should preserve the file
offset and use of the seek in the above feels iffy.
#include <stdlib.h>
#include <sys/types.h>
#include <fcntl.h>
#include <unistd.h>
int main(int ac, char **av)
{
int fd = open("testfile", O_CREAT|O_WRONLY, 0666);
if (fd < 0)
exit(1);
write(fd, "abcdefghijklmnopqrstuvwxyz\n", 27);
lseek(fd, 7, SEEK_SET);
write(fd, "H", 1);
ftruncate(fd, 24);
write(fd, "IJ", 2);
close(fd);
exit(0);
}
The only two in-tree users of ftruncate() do not care about this, though.
They both seek to the beginning and then truncate to zero length.
^ permalink raw reply
* Re: [PATCH 08/17] Test for WIN32 instead of __MINGW32_
From: Marius Storm-Olsen @ 2009-09-14 19:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <7v1vm9cohd.fsf@alter.siamese.dyndns.org>
Junio C Hamano said the following on 14.09.2009 21:31:
> Marius Storm-Olsen <mstormo@gmail.com> writes:
>
>> From: Frank Li <lznuaa@gmail.com>
>>
>> The code which is conditional on MinGW32 is actually conditional on Windows.
>> Use the WIN32 symbol, which is defined by the MINGW32 and MSVC environments,
>> but not by Cygwin.
>>
>> Signed-off-by: Frank Li <lznuaa@gmail.com>
>> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
>
> The description sounds very sensible; I wish I read this one first, before
> wasting time scratching my head while reading 03/17. Shouldn't that patch
> be squashed into this one?
Heh, yeah, I agree. After patch got updated with suggestions from last
roll, it clearly makes more sense hear now. I'll update tomorrow. Thanks.
--
.marius
^ permalink raw reply
* Re: [PATCH 08/17] Test for WIN32 instead of __MINGW32_
From: Junio C Hamano @ 2009-09-14 19:31 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <8da5dcea21ce49b3a57fd224c088785a9f98e867.1252925290.git.mstormo@gmail.com>
Marius Storm-Olsen <mstormo@gmail.com> writes:
> From: Frank Li <lznuaa@gmail.com>
>
> The code which is conditional on MinGW32 is actually conditional on Windows.
> Use the WIN32 symbol, which is defined by the MINGW32 and MSVC environments,
> but not by Cygwin.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
> Signed-off-by: Marius Storm-Olsen <mstormo@gmail.com>
The description sounds very sensible; I wish I read this one first, before
wasting time scratching my head while reading 03/17. Shouldn't that patch
be squashed into this one?
> ---
> help.c | 2 +-
> pager.c | 4 ++--
> run-command.c | 8 ++++----
> run-command.h | 2 +-
> setup.c | 2 +-
> 5 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/help.c b/help.c
> index fd51b8e..e8db31f 100644
> --- a/help.c
> +++ b/help.c
> @@ -126,7 +126,7 @@ static int is_executable(const char *name)
> !S_ISREG(st.st_mode))
> return 0;
>
> -#ifdef __MINGW32__
> +#ifdef WIN32
> { /* cannot trust the executable bit, peek into the file instead */
> char buf[3] = { 0 };
> int n;
> diff --git a/pager.c b/pager.c
> index f416d38..86facec 100644
> --- a/pager.c
> +++ b/pager.c
> @@ -9,7 +9,7 @@
>
> static int spawned_pager;
>
> -#ifndef __MINGW32__
> +#ifndef WIN32
> static void pager_preexec(void)
> {
> /*
> @@ -72,7 +72,7 @@ void setup_pager(void)
> static const char *env[] = { "LESS=FRSX", NULL };
> pager_process.env = env;
> }
> -#ifndef __MINGW32__
> +#ifndef WIN32
> pager_process.preexec_cb = pager_preexec;
> #endif
> if (start_command(&pager_process))
> diff --git a/run-command.c b/run-command.c
> index 91f6d2e..cb006e7 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -75,7 +75,7 @@ fail_pipe:
>
> trace_argv_printf(cmd->argv, "trace: run_command:");
>
> -#ifndef __MINGW32__
> +#ifndef WIN32
> fflush(NULL);
> cmd->pid = fork();
> if (!cmd->pid) {
> @@ -315,7 +315,7 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
> return run_command(&cmd);
> }
>
> -#ifdef __MINGW32__
> +#ifdef WIN32
> static unsigned WINAPI run_thread(void *data)
> {
> struct async *async = data;
> @@ -331,7 +331,7 @@ int start_async(struct async *async)
> return error("cannot create pipe: %s", strerror(errno));
> async->out = pipe_out[0];
>
> -#ifndef __MINGW32__
> +#ifndef WIN32
> /* Flush stdio before fork() to avoid cloning buffers */
> fflush(NULL);
>
> @@ -360,7 +360,7 @@ int start_async(struct async *async)
>
> int finish_async(struct async *async)
> {
> -#ifndef __MINGW32__
> +#ifndef WIN32
> int ret = wait_or_whine(async->pid, "child process", 0);
> #else
> DWORD ret = 0;
> diff --git a/run-command.h b/run-command.h
> index 0c00b25..fb34209 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -70,7 +70,7 @@ struct async {
> int (*proc)(int fd, void *data);
> void *data;
> int out; /* caller reads from here and closes it */
> -#ifndef __MINGW32__
> +#ifndef WIN32
> pid_t pid;
> #else
> HANDLE tid;
> diff --git a/setup.c b/setup.c
> index e3781b6..029371e 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -41,7 +41,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)
> const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
> {
> static char path[PATH_MAX];
> -#ifndef __MINGW32__
> +#ifndef WIN32
> if (!pfx || !*pfx || is_absolute_path(arg))
> return arg;
> memcpy(path, pfx, pfx_len);
> --
> 1.6.2.1.418.g33d56.dirty
^ permalink raw reply
* Re: Re: Tracking remote branches
From: Heiko Voigt @ 2009-09-14 19:19 UTC (permalink / raw)
To: Thiago Farina; +Cc: Nicolas Sebrecht, git
In-Reply-To: <a4c8a6d00909141130o628a069fu4c489b25f95f543d@mail.gmail.com>
On Mon, Sep 14, 2009 at 03:30:10PM -0300, Thiago Farina wrote:
> On Mon, Sep 14, 2009 at 2:01 AM, Nicolas Sebrecht <nicolas.s.dev@gmx.fr> wrote:
> > The 13/09/09, Thiago Farina wrote:
> > $ git checkout -t origin/maint
> >
> >> OK, now I switched to maint branch. And then I did:
> >> $ git status
> >>
> >> # On branch maint
> >> # Changes to be committed
> >> # (use "git reset HEAD <file>..." to unstage)
> >> #
> >> # new file: git-remote-curl
> >> # new file: git-replace
> >> #
> >>
> >> What happened here? What I have to do now?
>
> Why these files has this status? Is supposed to me to do something
> about that when this situation occurs?
I just checked and this is because you probably made a build on the
master branch which creates these files. They are untracked but ignored
through the .gitignore file in the master branch.
Now the situation is that the .gitignore on the current 'maint' branch
does not have these files ignored because they are quite new.
In this situation you can do a
git clean -f
which gets rid of all untracked but not ignored files (all files that
git status shows as new) or
make clean
while you are still on the master branch.
cheers Heiko
^ permalink raw reply
* Re: [PATCH 15/17] Add MSVC to Makefile
From: Marius Storm-Olsen @ 2009-09-14 19:09 UTC (permalink / raw)
To: Alex Riesen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <81b0412b0909140706s32e888ddwe4bbd668c3a7ccfd@mail.gmail.com>
Alex Riesen said the following on 14.09.2009 16:06:
> On Mon, Sep 14, 2009 at 15:11, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>> @@ -1327,7 +1379,7 @@ strip: $(PROGRAMS) git$X
>> git.o: git.c common-cmds.h GIT-CFLAGS
>> $(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
>> '-DGIT_HTML_PATH="$(htmldir_SQ)"' \
>> - $(ALL_CFLAGS) -c $(filter %.c,$^)
>> + $(ALL_CFLAGS) -o git.o -c $(filter %.c,$^)
>
> Why do you use "-o git.o" instead of "-o $@"?
No reason really. I'll resend tomorrow.
--
.marius
^ permalink raw reply
* Re: [msysGit] [PATCH 06/17] mingw.c: Use the O_BINARY flag to open files
From: Alexey Borzenkov @ 2009-09-14 19:06 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <160ceafba5f58cfcadb13380e3d8ef724009b700.1252925290.git.mstormo@gmail.com>
On Mon, Sep 14, 2009 at 5:11 PM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
> On Windows, binary files must be opened using the O_BINARY flag for
> untranslated mode. MinGW does this for us automatically, but Microsoft
> Visual C++ does not.
Wouldn't it be better to set explicitly set _fmode = _O_BINARY when
_MSC_VER is defined? _CRT_fmode is mingw specific, that in addition to
setting _fmode also sets mode on stdin/stdout/stderr (see
http://www.koders.com/c/fid2B507DE94598397757D615EDF52267578AD838A2.aspx).
Current patch will not set binary mode for files opened with fopen, as
well as stdin/stdout/stderr. One catch might be that declaring it like
"unsigned int _fmode = _O_BINARY" might not work, it seems that you
are supposed to set it somewhere in the code (see
http://msdn.microsoft.com/en-us/library/ee2849wt(VS.80).aspx). Perhaps
implementing a function like _mingw32_init_fmode in crc1.c in
mingw-runtime and calling it early in main() would be better?
Thanks,
Alexey.
^ permalink raw reply
* [PATCH] git gui: make current branch default in "remote delete branch" merge check
From: Heiko Voigt @ 2009-09-14 19:05 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
---
git-gui/lib/remote_branch_delete.tcl | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/git-gui/lib/remote_branch_delete.tcl b/git-gui/lib/remote_branch_delete.tcl
index 4e02fc0..9b119a7 100644
--- a/git-gui/lib/remote_branch_delete.tcl
+++ b/git-gui/lib/remote_branch_delete.tcl
@@ -248,6 +248,8 @@ method _write_url {args} { set urltype url }
method _write_check_head {args} { set checktype head }
method _write_head_list {args} {
+ global current_branch
+
$head_m delete 0 end
foreach abr $head_list {
$head_m insert end radiobutton \
@@ -256,7 +258,11 @@ method _write_head_list {args} {
-variable @check_head
}
if {[lsearch -exact -sorted $head_list $check_head] < 0} {
- set check_head {}
+ if {[lsearch -exact -sorted $head_list $current_branch] < 0} {
+ set check_head {}
+ } else {
+ set check_head $current_branch
+ }
}
}
--
1.6.5.rc1.10.g20f34
^ permalink raw reply related
* Re: [PATCH 14/17] Add ftruncate implementation for MSVC
From: Marius Storm-Olsen @ 2009-09-14 19:03 UTC (permalink / raw)
To: Alex Riesen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <81b0412b0909140718i7d2c4e3cpc96477f8374d80b4@mail.gmail.com>
Alex Riesen said the following on 14.09.2009 16:18:
> On Mon, Sep 14, 2009 at 15:11, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>> +int ftruncate(int fd, __int64 length)
>> +{
>> + HANDLE fh = (HANDLE)_get_osfhandle(fd);
>> + if (!fh || _lseeki64(fd, length, SEEK_SET))
>
> According to MSDN, error is manifested by returning INVALID_HANDLE_VALUE (–1):
You're absolutely right. I was too deep in Posix-land when writing this.
Updated patch will follow tomorrow.
--
.marius
^ permalink raw reply
* Re: [PATCH 01/17] Avoid declaration after statement
From: Marius Storm-Olsen @ 2009-09-14 19:00 UTC (permalink / raw)
To: Alex Riesen; +Cc: Frank Li, Johannes.Schindelin, msysgit, git
In-Reply-To: <81b0412b0909140710y586f0464vc7a3b08460f453d@mail.gmail.com>
Alex Riesen said the following on 14.09.2009 16:10:
> On Mon, Sep 14, 2009 at 16:04, Frank Li <lznuaa@gmail.com> wrote:
>>>> Microsoft Visual C++ does not understand this C99 style
>>>>
>>> But you don't use the C++ compiler (which does understand that
>>> syntax).
>>>
>> Microsoft Visual C++ is product name. If you think it is confuse,
>> it can change to "MSVC"
>
> I do. OTOH, some people actually use a "product" to compile
> "software", so maybe it is okay.
Ok, I'll s/Microsoft Visual C++/MSVC/ and resend tomorrow, just to be
consistent with the other patches.
--
.marius
^ permalink raw reply
* Re: Tracking remote branches
From: Thiago Farina @ 2009-09-14 18:30 UTC (permalink / raw)
To: Nicolas Sebrecht; +Cc: git
In-Reply-To: <20090914050145.GA12985@vidovic>
On Mon, Sep 14, 2009 at 2:01 AM, Nicolas Sebrecht <nicolas.s.dev@gmx.fr> wrote:
> The 13/09/09, Thiago Farina wrote:
>> Hi,
>>
>> I know that I could do simply, instead of above:
>> $ git checkout --track -b maint origin/maint
>
> Or
> $ git checkout -t origin/maint
>
>> OK, now I switched to maint branch. And then I did:
>> $ git status
>>
>> # On branch maint
>> # Changes to be committed
>> # (use "git reset HEAD <file>..." to unstage)
>> #
>> # new file: git-remote-curl
>> # new file: git-replace
>> #
>>
>> What happened here? What I have to do now?
>
> You've probably added these files to the index, mistakenly. If you don't
> have uncommited changes, try :
>
> git checkout -f master
> git branch -D maint
> git checkout -t origin/maint
> git status
>
> The files should now be shown as "Untracked files".
Yeah, now the files has this status. But I don't have anything related
with these files, I didn't add these files.
Why these files has this status? Is supposed to me to do something
about that when this situation occurs?
^ permalink raw reply
* Re: [PATCH 14/17] Add ftruncate implementation for MSVC
From: Alex Riesen @ 2009-09-14 14:18 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <c95a367520435feec0833d2d0edc33d86cf939a8.1252925290.git.mstormo@gmail.com>
On Mon, Sep 14, 2009 at 15:11, Marius Storm-Olsen <mstormo@gmail.com> wrote:
> +int ftruncate(int fd, __int64 length)
> +{
> + HANDLE fh = (HANDLE)_get_osfhandle(fd);
> + if (!fh || _lseeki64(fd, length, SEEK_SET))
According to MSDN, error is manifested by returning INVALID_HANDLE_VALUE (–1):
http://msdn.microsoft.com/en-us/library/ks2530z6.aspx
^ permalink raw reply
* Re: [PATCH 01/17] Avoid declaration after statement
From: Alex Riesen @ 2009-09-14 14:10 UTC (permalink / raw)
To: Frank Li; +Cc: Marius Storm-Olsen, Johannes.Schindelin, msysgit, git
In-Reply-To: <1976ea660909140704r4b730002o5d3f42463b57828d@mail.gmail.com>
On Mon, Sep 14, 2009 at 16:04, Frank Li <lznuaa@gmail.com> wrote:
>>>
>>> Microsoft Visual C++ does not understand this C99 style
>>>
>>
>> But you don't use the C++ compiler (which does understand that syntax).
>>
>
> Microsoft Visual C++ is product name. If you think it is confuse, it
> can change to "MSVC"
>
I do. OTOH, some people actually use a "product" to compile "software",
so maybe it is okay.
^ permalink raw reply
* Re: [PATCH 15/17] Add MSVC to Makefile
From: Alex Riesen @ 2009-09-14 14:06 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <abaa803c30801c570f4b8e1baf6648a482bf295d.1252925290.git.mstormo@gmail.com>
On Mon, Sep 14, 2009 at 15:11, Marius Storm-Olsen <mstormo@gmail.com> wrote:
> @@ -1327,7 +1379,7 @@ strip: $(PROGRAMS) git$X
> git.o: git.c common-cmds.h GIT-CFLAGS
> $(QUIET_CC)$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
> '-DGIT_HTML_PATH="$(htmldir_SQ)"' \
> - $(ALL_CFLAGS) -c $(filter %.c,$^)
> + $(ALL_CFLAGS) -o git.o -c $(filter %.c,$^)
Why do you use "-o git.o" instead of "-o $@"?
^ permalink raw reply
* Re: [PATCH 01/17] Avoid declaration after statement
From: Frank Li @ 2009-09-14 14:04 UTC (permalink / raw)
To: Alex Riesen; +Cc: Marius Storm-Olsen, Johannes.Schindelin, msysgit, git
In-Reply-To: <81b0412b0909140659p2472c026gaece8b9388fb21d6@mail.gmail.com>
>>
>> Microsoft Visual C++ does not understand this C99 style
>>
>
> But you don't use the C++ compiler (which does understand that syntax).
>
Microsoft Visual C++ is product name. If you think it is confuse, it
can change to "MSVC"
^ permalink raw reply
* Re: [PATCH 01/17] Avoid declaration after statement
From: Marius Storm-Olsen @ 2009-09-14 14:01 UTC (permalink / raw)
To: Alex Riesen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <81b0412b0909140659p2472c026gaece8b9388fb21d6@mail.gmail.com>
Alex Riesen said the following on 14.09.2009 15:59:
> On Mon, Sep 14, 2009 at 15:11, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>> From: Frank Li <lznuaa@gmail.com>
>>
>> Microsoft Visual C++ does not understand this C99 style
>
> But you don't use the C++ compiler (which does understand that syntax).
It doesn't change the name of the compiler which is called "Microsoft
Visual C++". We're just using the C part of it :)
--
.marius
^ permalink raw reply
* Re: [PATCH 01/17] Avoid declaration after statement
From: Alex Riesen @ 2009-09-14 13:59 UTC (permalink / raw)
To: Marius Storm-Olsen; +Cc: Johannes.Schindelin, msysgit, git, lznuaa
In-Reply-To: <1b36d8e041c9c1a6dc68f9cc3919fcf91b7a9ac6.1252925290.git.mstormo@gmail.com>
On Mon, Sep 14, 2009 at 15:11, Marius Storm-Olsen <mstormo@gmail.com> wrote:
> From: Frank Li <lznuaa@gmail.com>
>
> Microsoft Visual C++ does not understand this C99 style
>
But you don't use the C++ compiler (which does understand that syntax).
^ permalink raw reply
* Re: [PATCH 02/17] boolean is a typedef under MSVC, so rename variable to 'i_boolean'
From: Marius Storm-Olsen @ 2009-09-14 13:41 UTC (permalink / raw)
To: git; +Cc: Johannes.Schindelin, msysgit, lznuaa
In-Reply-To: <7786dfa1b74b5c4974bcf2018d3991c215a9466f.1252925290.git.mstormo@gmail.com>
Marius Storm-Olsen said the following on 14.09.2009 15:11:
> With MSVC 'boolean' is already a typedef to unsigned char in the
> rpcndr.h in the SDK. So, just rename to 'i_boolean' to avoid
> conflicts in this one file, instead of any include magic.
This patch can actually be dropped, since patch 12 makes the windows.h
include "Lean and Mean", and rpcndr.h is not indirectly include
anymore. Sorry for not spotting this before sending the series..
--
.marius
^ permalink raw reply
* Re: [PATCH 2/2] remove NORETURN from function pointers
From: Johannes Sixt @ 2009-09-14 13:37 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: Jeff King, git, Erik Faye-Lund
In-Reply-To: <40aa078e0909140626k63b9de2fu8c9411baf8200da6@mail.gmail.com>
Erik Faye-Lund schrieb:
> On Mon, Sep 14, 2009 at 3:19 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> "The" warning? Not "the two" warnings? Then I suggest to stop here; MSVC
>> is only half-competent with regards to noreturn.
>
> There was only one warning in this regard on MSVC - the one about
> unreachable code. And yes, MSVC is only half-competent, but it seems
> it's competence is in the half that matters in our case.
>
> Do you suggest to stop the patch-series, or to stop the testing?
My suggestion was about stopping the patch series.
But thinking a bit more about it, I can imagine that there are calls to
die() that, if it is not marked noreturn, could trigger other warnings
with MSVC. That would be annoying, and it's better to mark it noreturn.
So I withdraw my suggestion to stop :-)
-- Hannes
^ permalink raw reply
* Re: [PATCH 2/2] remove NORETURN from function pointers
From: Erik Faye-Lund @ 2009-09-14 13:26 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Jeff King, git, Erik Faye-Lund
In-Reply-To: <4AAE42F2.30304@viscovery.net>
On Mon, Sep 14, 2009 at 3:19 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> "The" warning? Not "the two" warnings? Then I suggest to stop here; MSVC
> is only half-competent with regards to noreturn.
There was only one warning in this regard on MSVC - the one about
unreachable code. And yes, MSVC is only half-competent, but it seems
it's competence is in the half that matters in our case.
Do you suggest to stop the patch-series, or to stop the testing? I'd
prefer having NORETURN for die() etc in MSVC-builds, as it allows the
compiler to generate better code. I'm prefectly fine about not having
NORETURN for the function pointers. GCC should be competent enough to
catch (very theoretical) errors, and we get the nice speed-improvement
on MSVC. I don't see the down-side.
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ 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