* Re: [PATCH 2/2 (v2)] reset: make the output more user-friendly.
From: Matthieu Moy @ 2009-08-17 17:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <vpq7hxeu4un.fsf@bauges.imag.fr>
[ back from holiday ]
Any opinion/update on this? I don't think I got any reply ...
Thanks,
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>>
>>> cat > expect << EOF
>>> -file2: locally modified
>>> +Unstaged changes after reset:
>>> +M file2
>>
>> It simply feels backwards when plumbing output says something in human
>> language (e.g. "needs update") while Porcelain output spits out a cryptic
>> M or U. If the goal is human-readability and user-friendliness,
>
> The goal here is just consistency.
>
> And I do consider 'git diff --name-status' as porcelain.
>
>> shouldn't we rather say:
>>
>> Path with local modifications:
>> file2
>>
>> or something?
>
> Why not, but if we do, we should also remove this "M" from other
> places. It was already there in one error message given by 'git
> rebase' in a non-clean tree (and you just accepted a patch giving the
> same output for another one).
--
Matthieu
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Joshua Jensen @ 2009-08-17 18:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Frank Li, git, msysgit
In-Reply-To: <alpine.DEB.1.00.0908171840450.4991@intel-tinevez-2-302>
----- Original Message -----
From: Johannes Schindelin
Date: 8/17/2009 10:49 AM
> How about this instead?
>
> Work around Microsoft Visual C++ not having va_copy()
>
> In winansi.c, Git wants to know the length of the formatted string
> so it can allocate enough space for it. But Microsoft Visual C++
> does not have va_copy(), so we have to guess
I did not look at the surrounding code, but could Microsoft's C runtime
extension _vscprintf, which returns the number of characters in the
formatted string, be of use here?
Josh
^ permalink raw reply
* Re: Using VC build git (split patch)
From: Marius Storm-Olsen @ 2009-08-17 18:22 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysGit, Johannes Schindelin
In-Reply-To: <1976ea660908170613ibb9a0fdr7ba630671a6b735f@mail.gmail.com>
Frank Li said the following on 17.08.2009 15:13:
> Thanks Dscho for reviewing my patch patient.
> I split my patch and read SumbittingPatchs.
>
> I try to use git format-patch -M --stdout origin/master | git
> imap-send to send patch directly according to guide.
> But imap-send has not ported to msysgit.
Hi Frank,
Could you please also update your repo at repo.or.cz, then it'll be
easier if anyone wants to help you in the process of streamlining the
patch series?
--
.marius
^ permalink raw reply
* Re: [PATCH 10/11] Add MSVC Project file
From: Paolo Bonzini @ 2009-08-17 18:16 UTC (permalink / raw)
To: Erik Faye-Lund; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <40aa078e0908171040g5718a809o88b093fe5a4a0e28@mail.gmail.com>
On 08/17/2009 07:40 PM, Erik Faye-Lund wrote:
> On Mon, Aug 17, 2009 at 7:34 PM, Paolo Bonzini<bonzini@gnu.org> wrote:
>> Also, a cleaner XML without verbosities like
>>
>> +<Tool
>> + Name="VCMIDLTool"
>> + />
>>
>> would make the patch easier to review.
>
> ...but will it make it more annoying to maintain in the long run? It
> might be painful to work with a mixture of hand-written and
> msdev-written XML. Of course, if we get some scripts in place to
> generate the vcproj-files this might not be a problem...
Yes, my remark applied only if the generation was scripted. Otherwise,
it would be an annoyance in addition to being subject to bitrot.
Paolo
^ permalink raw reply
* Re: RFC for 1.7: Do not checkout -b master origin/master on clone
From: Matthieu Moy @ 2009-08-17 17:45 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List
In-Reply-To: <4A818B90.9050206@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> - git clone does not set up any local branches at all
> - git svn fetch does not set up any local branches at all
If you don't set up a local branch, it means either
- You start with a detached HEAD, or
- You start with no working tree at all.
In both cases, you introduce more trouble for the beginner, just more
notions to learn before being able to do the very first step.
And you also break a good property of Git: in Git, you can use the
"one repo per branch" or the "multiple branches in a repository" the
way you want.
Personnally, I rarely have more than one branch for me (but I
appreciate very much having one branch per developer). And this way, I
can just forget that Git has multiple branches in a repo. When I want
to work on a project, I clone it, I hack, I commit, and then I
send the result (git push, git svn dcommit, git send-email, depending
on the project). In this workflow, I almost never have to use "git
branch", "git checkout", "git merge", it works, and Git does the right
thing for me.
--
Matthieu
^ permalink raw reply
* Re: [PATCH 10/11] Add MSVC Project file
From: Erik Faye-Lund @ 2009-08-17 17:40 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <4A899495.8050902@gnu.org>
On Mon, Aug 17, 2009 at 7:34 PM, Paolo Bonzini<bonzini@gnu.org> wrote:
> Also, a cleaner XML without verbosities like
>
> + <Tool
> + Name="VCMIDLTool"
> + />
>
> would make the patch easier to review.
...but will it make it more annoying to maintain in the long run? It
might be painful to work with a mixture of hand-written and
msdev-written XML. Of course, if we get some scripts in place to
generate the vcproj-files this might not be a problem...
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: [PATCH 10/11] Add MSVC Project file
From: Paolo Bonzini @ 2009-08-17 17:34 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit, Johannes.Schindelin
In-Reply-To: <1250525103-5184-5-git-send-email-lznuaa@gmail.com>
I don't know if this is in any way feasible (does not seem to hard
actually), but I think these files should be automatically generated.
Otherwise, bitrot is going to happen in zero time.
Also, a cleaner XML without verbosities like
+ <Tool
+ Name="VCMIDLTool"
+ />
would make the patch easier to review.
I agree with dscho on the non-builtins.
Paolo
^ permalink raw reply
* Re: Linus' sha1 is much faster!
From: Giuseppe Scrivano @ 2009-08-17 17:32 UTC (permalink / raw)
To: Steven Noonan
Cc: Pádraig Brady, Bug-coreutils, Linus Torvalds,
Git Mailing List
In-Reply-To: <f488382f0908170844h649126efxb27f87d7b319961b@mail.gmail.com>
Hi,
These are the results I reported (median of 5 plus an additional not
considered first run) on the Steve Reid's SHA1 implementation using the
same flags to the compiler that I used for previous tests.
GCC 4.3.3: real 0m2.627s
GCC 4.4.1: real 0m3.742s
In both cases it showed to be slower than other implementations I have
already tried.
Additional note: as for gnulib SHA1, GCC 4.4.1 produced slower code than
GCC 4.3.3.
Cheers,
Giuseppe
Steven Noonan <steven@uplinklabs.net> writes:
>
> Interesting. I compared Linus' implementation to the public domain one
> by Steve Reid[1], which is used in OpenLDAP and a few other projects.
> Anyone with some experience testing these kinds of things in a
> statistically sound manner want to try it out? In my tests, I got
> this:
>
> (average of 5 runs)
> Linus' sha1: 283MB/s
> Steve Reid's sha1: 305MB/s
>
> - Steven
>
> [1] http://gpl.nas-central.org/SYNOLOGY/x07-series/514_UNTARED/source/openldap-2.3.11/libraries/liblutil/sha1.c
^ permalink raw reply
* Re: git http-push and MKCOL error (22/409)
From: Thomas Schlichter @ 2009-08-17 17:28 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Junio C Hamano, willievu, Sean Davis, git
In-Reply-To: <20090817170943.180dbedd.rctay89@gmail.com>
Hi,
Am Montag 17 August 2009 11:09:43 schrieb Tay Ray Chuan:
> Subject: [PATCH] http.c: don't assume that urls don't end with slash
>
> Make append_remote_object_url() (and by implication,
> get_remote_object_url) use end_url_with_slash() to ensure that the url
> ends with a slash.
>
> Previously, they assumed that the url did not end with a slash and
> as a result appended a slash, sometimes errorneously.
>
> This fixes an issue introduced in 5424bc5 ("http*: add helper methods
> for fetching objects (loose)"), where the append_remote_object_url()
> implementation in http-push.c, which assumed that urls end with a
> slash, was replaced by another one in http.c, which assumed urls did
> not end with a slash.
>
> The above issue was raised by Thomas Schlichter:
>
> http://marc.info/?l=git&m=125043105231327
>
> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Thank you very much!
This patch indeed fixes my problem.
You may add my
Tested-by: Thomas Schlichter <thomas.schlichter@web.de>
if you'd like to. I don't know how this is handled in the git-tree...
Is there a plan in which version this patch will be released? Will there be a
1.6.4.1 or is 1.6.5 next?
Kind regards,
Thomas Schlichter
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Erik Faye-Lund @ 2009-08-17 17:26 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <40aa078e0908171002j4b610fe4j34a4e7d3081a9efa@mail.gmail.com>
On Mon, Aug 17, 2009 at 7:02 PM, Erik Faye-Lund<kusmabite@googlemail.com> wrote:
> Are you sure va_copy is always a preprocessor symbol?
According to the following forum-post we are:
http://www.velocityreviews.com/forums/showpost.php?p=1689162&postcount=2
However, I decided to dig a bit further, so I had a look at the public
draft spec at http://www.open-std.org/JTC1/SC22/WG14/www/docs/n1256.pdf,
section 7.15.1:
"The va_start and va_arg macros described in this subclause shall be implemented
as macros, not functions. It is unspecified whether va_copy and va_end
are macros or
identifiers declared with external linkage."
I don't have access (that I know of) to the finalized spec, but it
looks sketchy to me to depend on va_copy being implemented as a macro
given this wording.
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: Linus' sha1 is much faster!
From: Paolo Bonzini @ 2009-08-17 17:20 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: George Spelvin, bdonlan, johnflux, P, art.08.09, git
In-Reply-To: <alpine.LFD.2.00.0908171228570.6044@xanadu.home>
> my first version of
> get_be32() was a macro that did this:
>
> #define SHA_SRC(t) \
> ({ unsigned char *__d = (unsigned char *)&data[t]; \
> (__d[0]<< 24) | (__d[1]<< 16) | (__d[2]<< 8) | (__d[3]<< 0); })
>
> With such a construct, gcc would always allocate a register to hold __d
> and then dereference that with an offset from 0 to 3. Whereas:
>
> #define SHA_SRC(t) \
> ({ unsigned char *__d = (unsigned char *)data; \
> (__d[(t)*4 + 0]<< 24) | (__d[(t)*4 + 1]<< 16) | \
> (__d[(t)*4 + 2]<< 8) | (__d[(t)*4 + 3]<< 0); })
>
> does produce optimal assembly as only the register holding the data
> pointer is dereferenced with the absolute byte offset. I suspect your
> usage of inline functions has the same effect as the first SHA_SRC
> definition above.
Yes, that's what happens.
Paolo
^ permalink raw reply
* Re: [PATCH 11/11] Add gitignore and ReadMe file for MSVC Build
From: Johannes Schindelin @ 2009-08-17 17:16 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525340-412-1-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Add ReadMe to user how to build git with MSVC.
> Add gitignore to filter out files created file by MSVC.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
Please use the plural form "files" where appropriate.
> diff --git a/compat/vcbuild/ReadMe b/compat/vcbuild/ReadMe
> new file mode 100644
> index 0000000..47c089e
> --- /dev/null
> +++ b/compat/vcbuild/ReadMe
> @@ -0,0 +1,13 @@
> +The Steps of Build Git with VS2008
> +
> +1. Create VC Build Environment.
> +
> + git clone git://repo.or.cz/gitbuild.git
> +
> + gitbuild include VS solution file and library such as zlib.
How, if you do not have Git?
> +2. Get Submodule
> +
> + git submodule update
This is missing '--init'.
Ciao,
Dscho
^ permalink raw reply
* Re: sparse support in pu
From: Johannes Schindelin @ 2009-08-17 17:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: Peter Harris, Nguyen Thai Ngoc Duy, Johannes Sixt, skillzero, git
In-Reply-To: <7v7hx2xw6p.fsf@alter.siamese.dyndns.org>
Hi,
On Mon, 17 Aug 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> That's funny. I have a git tree that would benefit from sparse checkout.
> >> I know which path I _don't_ want. Specifying all the paths I want would
> >> be a rather longer (and more error-prone) list. I suspect it would be
> >> best to support both.
> >
> > Yes, I agree, but the common case is for people to know what they are
> > working on, right?
>
> I either may be working on Documentation/ (I know I do not care about
> everthing else so I can afford to list Makefile and Documentation/ in
> the "interesting" list), or I either may be working on code (I do not
> know how many code directories there are, and do not care to list them,
> but I know I do not need Documentation/ and contrib/). You need both
> ways.
>
> And with .gitignore syntax you can have both.
I was fully aware of this. It was the only reason I proposed the
.gitignore syntax.
My point was that the direction should be dictated by convenience to those
who need the feature most.
And maybe by asking yourself the question "what would I understand by
'sparse files'?"
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 10/11] Add MSVC Project file
From: Johannes Schindelin @ 2009-08-17 17:11 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-5-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Add libgit.vcproj to build common library.
> Add git.vcproj to build git program.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
The commit subject should read "... files", as you add two files, not one.
I hope that somebody else is going to review this patch...
And don't you lack .vcproj files for the non-builtins?
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 09/11] Add MSVC porting header files.
From: Johannes Schindelin @ 2009-08-17 17:09 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-4-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Add unix head file, dirent.h, unistd.h and time.h
These are copied from somewhere. From where? What is the license?
> Add MSVC special porting head file msvc.h and msvc.c.
This is added by you. Logically, that should be a separate patch.
> diff --git a/compat/msvc.h b/compat/msvc.h
> new file mode 100644
> index 0000000..6071565
> --- /dev/null
> +++ b/compat/msvc.h
> @@ -0,0 +1,95 @@
> +#ifndef __MSVC__HEAD
> +#define __MSVC__HEAD
> +
> +#define WINVER 0x0500
> +#define _WIN32_WINNT 0x0500
> +#define _WIN32_WINDOWS 0x0410
> +#define _WIN32_IE 0x0700
> +#define NTDDI_VERSION NTDDI_WIN2KSP1
> +#include <winsock2.h>
> +
> +/*Configuration*/
> +
> +#define NO_PREAD
> +#define NO_OPENSSL
> +#define NO_LIBGEN_H
> +#define NO_SYMLINK_HEAD
> +#define NO_IPV6
> +#define NO_SETENV
> +#define NO_UNSETENV
> +#define NO_STRCASESTR
> +#define NO_STRLCPY
> +#define NO_MEMMEM
> +#define NO_C99_FORMAT
> +#define NO_STRTOUMAX
> +#define NO_MKDTEMP
> +#define NO_MKSTEMPS
> +
> +#define RUNTIME_PREFIX
> +#define NO_ST_BLOCKS_IN_STRUCT_STAT
> +#define NO_NSEC
> +#define USE_WIN32_MMAP
> +#define USE_NED_ALLOCATOR
> +
> +#define NO_REGEX
> +
> +#define NO_SYS_SELECT_H
> +#define NO_PTHEADS
> +#define HAVE_STRING_H 1
> +#define STDC_HEADERS
> +#define NO_ICONV
These would normally be defined in the Makefile. You might want to state
that in a comment.
Or maybe move the definitions (along with vsnprintf) to the .vcproj file,
which is the logical pendant of the Makefile?
> +#define inline __inline
> +#define __inline__ __inline
These definitions are unrelated to the surrounding ones; please move them
elsewhere.
> +
> +#define SNPRINTF_RETURNS_BOGUS
> +
> +#define SHA1_HEADER "mozilla-sha1\\sha1.h"
> +
> +#define ETC_GITCONFIG "%HOME%"
> +
> +#define NO_PTHREADS
> +#define NO_CURL
> +
> +
> +#define NO_STRTOUMAX
> +#define REGEX_MALLOC
> +
> +
> +#define GIT_EXEC_PATH "bin"
> +#define GIT_VERSION "1.6"
> +#define BINDIR "bin"
> +#define PREFIX "."
> +#define GIT_MAN_PATH "man"
> +#define GIT_INFO_PATH "info"
> +#define GIT_HTML_PATH "html"
> +#define DEFAULT_GIT_TEMPLATE_DIR "templates"
> +
> +#define NO_STRLCPY
> +#define NO_UNSETENV
> +#define NO_SETENV
Would these NO_ definitions not _love_ to be close to their siblings?
What is the reason for those empty lines? Their placement and amount look
rather arbitrary to me.
> +#define strdup _strdup
> +#define read _read
> +#define close _close
> +#define dup _dup
> +#define dup2 _dup2
> +#define strncasecmp _strnicmp
> +#define strtoull _strtoui64
vsnprintf could go right here.
> +#define __attribute__(x)
The two inline definitions could go right here.
> +static __inline int strcasecmp (const char *s1, const char *s2)
> +{
> + int size1=strlen(s1);
> + int sisz2=strlen(s2);
> +
> + return _strnicmp(s1,s2,sisz2>size1?sisz2:size1);
> +}
> +
> +#include "compat/mingw.h"
> +#undef ERROR
> +#undef stat
> +#define stat(x,y) mingw_lstat
> +#define stat _stat64
> +#endif
Looks much nicer now, thanks!
Ciao,
Dscho
^ permalink raw reply
* Re: "make quick-install-man" broke recently
From: Junio C Hamano @ 2009-08-17 17:09 UTC (permalink / raw)
To: Linus Torvalds
Cc: Junio C Hamano, Jacob Helwig, Kjetil Barvik, Randal L. Schwartz,
git@vger.kernel.org
In-Reply-To: <alpine.LFD.2.01.0908170932390.3162@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Grr. Now 'check_path()' is no longer something generically useful.
>
> Could you perhaps instead only change 'checkout_entry()' to do this hack,
> and leave 'check_path()' as a generic replacement for "lstat()" that
> doesn't follow symlinks?
Given that non-empty base_dir is only used for "checkout-index --prefix",
iow, the "path" internally used by git and fed to our symlink.c cache are
supposed to be always relative to the work tree, I think that may be a
good thing to do in the short-term.
But only if we won't add any more like "checkout-index --prefix". If you
want to implement "git checkout --prefix=over-there/" and if you want to
call check_path() directly (iow not as a part of callchain from
checkout_entry()) while doing so, for example, you would regret keeping
the check_path() function unaware of base_dir, as you would reintroduce
the same bug.
I thought about getting rid of base_dir from struct checkout by running
create_directories() in checkout-index and chdir(2) there, because this is
a very special case codepath anyway. I actually haven't tried it, but it
probably will have bad interactions with the way we find $GIT_DIR.
^ permalink raw reply
* Re: Linus' sha1 is much faster!
From: Nicolas Pitre @ 2009-08-17 17:06 UTC (permalink / raw)
To: George Spelvin; +Cc: bdonlan, johnflux, P, art.08.09, git, torvalds
In-Reply-To: <20090817072315.4314.qmail@science.horizon.com>
On Mon, 17 Aug 2009, George Spelvin wrote:
> If it helps anyone resolve license issues, here's a from-FIPS-180-2
> implementation that's placed in the public domain. That should be
> compatible with any license.
>
> It uses Linus's and Artur's performance ideas, and some of Linus' macro
> ideas (in the rotate implementation), but tries to be textually different.
> Is there anything recognizable that anyone cares to clam copyright to?
I don't think this trick of making source code textually different from
another work while still intimately mimicking the same structure entitles
you to any copyright (or non copyright) claims over that other work. I
certainly wouldn't bet any dime for this standing up in court.
Otherwise anyone could grab any copyrighted source code and perform a
bunch of search-and-replace ops on it, and maybe some code reordering
for good measure, to be able to claim own copyright on it. It is
probably much safer to simply ask the people involved to agree with your
relicensing. And so far I don't see anyone with a stake in this
fiercely wanting to stick to a particular license.
> It's not quite 100% finished, as I haven't benchmarked it against Linus's
> code yet, but it's functionally correct.
>
> It's also clean with -W -Wall -Wextra.
Not if you try with the unaligned put_be32() as the destination pointer
is marked const.
As to the actual result on ARM... Well, the assembly _looks_ much worse
than Linus' version. It uses a stack frame of 152 bytes instead of 64
bytes. The resulting binary is also 6868 bytes large compared to 6180
bytes. Surprisingly, the performance is not that bad (the reason for
the underlined "looks" above) albeit still a bit worse, like 5% slower.
I was expecting much worse than that.
One possible reason for the bad assembly is probably due to the fact
that gcc is not smart enough to propagate constant address offsets
across different pointer types. For example, my first version of
get_be32() was a macro that did this:
#define SHA_SRC(t) \
({ unsigned char *__d = (unsigned char *)&data[t]; \
(__d[0] << 24) | (__d[1] << 16) | (__d[2] << 8) | (__d[3] << 0); })
With such a construct, gcc would always allocate a register to hold __d
and then dereference that with an offset from 0 to 3. Whereas:
#define SHA_SRC(t) \
({ unsigned char *__d = (unsigned char *)data; \
(__d[(t)*4 + 0] << 24) | (__d[(t)*4 + 1] << 16) | \
(__d[(t)*4 + 2] << 8) | (__d[(t)*4 + 3] << 0); })
does produce optimal assembly as only the register holding the data
pointer is dereferenced with the absolute byte offset. I suspect your
usage of inline functions has the same effect as the first SHA_SRC
definition above.
Also, wrt skipping the last 3 write back to the 16 word array... For
all the (limited) attempts I've made so far to do that, it always ended
up making things worse. I've yet to investigate why though.
Nicolas
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Erik Faye-Lund @ 2009-08-17 17:02 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <4A898B27.3040507@gnu.org>
On Mon, Aug 17, 2009 at 6:53 PM, Paolo Bonzini<bonzini@gnu.org> wrote:
> #ifndef va_copy
> #define va_copy(dst, src) ((dst) = (src))
> #endif
Are you sure va_copy is always a preprocessor symbol? How about
#ifdef _MSC_VER
#define va_copy(dst, src) ((dst) = (src))
#endif
instead? It'd make me sleep slightly better at night, at least ;)
--
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656
^ permalink raw reply
* Re: [PATCH 08/11] Place __stdcall to correct position.
From: Johannes Schindelin @ 2009-08-17 17:01 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-3-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> MSVC require __stdcall is between return value and function name.
> ALL Win32 API definition is as TYPE WINAPI function name
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about "... to the correct ..." and "MSVC requires _stdcall to be
between return value..." and "All Win32 API functions are declared with
the WINAPI attribute."?
> diff --git a/compat/mingw.c b/compat/mingw.c
> index d5fa0ed..0c9c793 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1146,7 +1146,7 @@ void mingw_open_html(const char *unixpath)
>
> int link(const char *oldpath, const char *newpath)
> {
> - typedef BOOL WINAPI (*T)(const char*, const char*, LPSECURITY_ATTRIBUTES);
> + typedef BOOL (WINAPI *T)(const char*, const char*, LPSECURITY_ATTRIBUTES);
An extra space slipped in, there.
> diff --git a/run-command.c b/run-command.c
> index df139da..423b506 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -295,12 +295,12 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
> }
>
> #if defined(__MINGW32__) || defined(_MSC_VER)
> -static __stdcall unsigned run_thread(void *data)
> +static unsigned __stdcall run_thread(void *data)
> {
> struct async *async = data;
> return async->proc(async->fd_for_proc, async->data);
> }
> -#endif
> +#endif /* __MINGW32__ || _MSC_VER */
I do not think this is necessary. There are only 5 lines wrapped into
those #ifdef guards, the developer should be able to see that far.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 07/11] Add O_BINARY flag to open flag at mingw.c
From: Johannes Schindelin @ 2009-08-17 16:58 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-2-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Windows will convert CR\LF and union code at text mode.
> Git doesn't like this. Add O_BINARY flag to open function
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about this instead?
mingw.c: Use the O_BINARY flag to open files
On Windows, non-text files must be opened using the O_BINARY flag.
MinGW does this for us automatically, but Microsoft Visual C++
does not.
Also, Johannes said that this would be a nice cleanup.
BTW what about fopen()?
Patch is obviously good.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Reece Dunn @ 2009-08-17 16:56 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Frank Li, git, msysgit, Johannes.Schindelin
In-Reply-To: <4A898B27.3040507@gnu.org>
2009/8/17 Paolo Bonzini <bonzini@gnu.org>:
> On 08/17/2009 06:04 PM, Frank Li wrote:
>>
>> MSVs have not implemented va_copy. remove va_copy at MSVC environment.
>> It will malloc buffer each time.
>
> ... but only a 257-byte buffer as dscho pointed out.
>
> In many places that do not have va_copy, a simple assignment works. And
> va_end is almost always a no-op. So what about
>
> #ifndef va_copy
> #define va_copy(dst, src) ((dst) = (src))
> #endif
>
> if it works on MSVC?
According to http://stackoverflow.com/questions/558223/vacopy-porting-to-visual-c
that should work.
- Reece
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Paolo Bonzini @ 2009-08-17 16:53 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit, Johannes.Schindelin
In-Reply-To: <1250525040-5868-1-git-send-email-lznuaa@gmail.com>
On 08/17/2009 06:04 PM, Frank Li wrote:
> MSVs have not implemented va_copy. remove va_copy at MSVC environment.
> It will malloc buffer each time.
... but only a 257-byte buffer as dscho pointed out.
In many places that do not have va_copy, a simple assignment works. And
va_end is almost always a no-op. So what about
#ifndef va_copy
#define va_copy(dst, src) ((dst) = (src))
#endif
if it works on MSVC?
Paolo
^ permalink raw reply
* Re: [PATCH 06/11] Add miss git-compat-util.h at regex.c and fnmatch.c Add git-compat-util.h to enable build at MSVC environment
From: Johannes Schindelin @ 2009-08-17 16:51 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525103-5184-1-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about this instead?
Add missing git-compat-util.h to regex.c and fnmatch.c
This will be needed to compile with Microsoft Visual C++.
> diff --git a/compat/fnmatch/fnmatch.c b/compat/fnmatch/fnmatch.c
> index 14feac7..5cbd49c 100644
> --- a/compat/fnmatch/fnmatch.c
> +++ b/compat/fnmatch/fnmatch.c
> @@ -16,6 +16,10 @@
> write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
> Boston, MA 02111-1307, USA. */
>
> +#ifdef _MSC_VER
> +#include "git-compat-util.h"
> +#endif
> +
Why not leave those #ifdef guards? Either they don't hurt, or they will
hurt Microsoft Visual C++, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 05/11] Remove va_copy at MSVC because there are va_copy.
From: Johannes Schindelin @ 2009-08-17 16:49 UTC (permalink / raw)
To: Frank Li; +Cc: git, msysgit
In-Reply-To: <1250525040-5868-1-git-send-email-lznuaa@gmail.com>
Hi,
On Tue, 18 Aug 2009, Frank Li wrote:
> MSVs have not implemented va_copy. remove va_copy at MSVC environment.
> It will malloc buffer each time.
>
> Signed-off-by: Frank Li <lznuaa@gmail.com>
How about this instead?
Work around Microsoft Visual C++ not having va_copy()
In winansi.c, Git wants to know the length of the formatted string
so it can allocate enough space for it. But Microsoft Visual C++
does not have va_copy(), so we have to guess.
The problem is the guessing part:
> diff --git a/compat/winansi.c b/compat/winansi.c
> index 9217c24..6091138 100644
> --- a/compat/winansi.c
> +++ b/compat/winansi.c
> @@ -310,9 +314,13 @@ static int winansi_vfprintf(FILE *stream, const char *format, va_list list)
> if (!console)
> goto abort;
>
> +#ifndef _MSC_VER
> va_copy(cp, list);
> len = vsnprintf(small_buf, sizeof(small_buf), format, cp);
> va_end(cp);
> +#else
> + len= sizeof(small_buf) ;
> +#endif
small_buf only is 256 bytes. How do you want to make sure that the
subsequent vsnprintf() is not writing outside of the buffer?
Also, you still miss a space between "len" and "=".
Ciao,
Dscho
^ permalink raw reply
* Re: sparse support in pu
From: James Pickens @ 2009-08-17 16:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Nguyen Thai Ngoc Duy, Johannes Sixt, skillzero, git
In-Reply-To: <alpine.DEB.1.00.0908171425410.4991@intel-tinevez-2-302>
On Mon, Aug 17, 2009, Johannes Schindelin<Johannes.Schindelin@gmx.de> wrote:
> The term 'phantom' is not specified at all. At least interested people on
> the mailing list know 'sparse'. But I agree that the naming is a major
> problem, hence my earlier (unanswered) call.
I don't particularly like 'phantom' either, but I haven't come up with any
good alternatives. 'sparse' seems fine to me, though it might make sense
to lengthen it to 'sparse-checkout'.
> However, I would find specifying what you do _not_ want in that file
> rather unintuitive, in the same leage as receive.denyNonFastForwards = no.
>
> If I want to have a sparse checkout, I know which files I _want_.
I agree; specifying which files you don't want seems backwards to me.
James
^ 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