Git development
 help / color / mirror / Atom feed
* Re: Interim maintainer tree
From: Jeff King @ 2009-10-04 13:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7viqevpjr3.fsf@alter.siamese.dyndns.org>

On Sun, Oct 04, 2009 at 02:54:08AM -0700, Junio C Hamano wrote:

> >   - silence gcc warning:
> >     http://article.gmane.org/gmane.comp.version-control.git/129485
> >     The warning is overly cautious, I think, but it is a dubious enough
> >     construct that it is probably worth fixing.
> 
> I cannot reach gmane now, but if this is about -Wextra, I'd rather not
> touch it before the release.  "comparison between signed and unsigned"
> tends to be excessive and IMNSHO it is crazy to use -Wextra and -Werror 
> together.

Actually, I mislabeled this. It is about a glibc run-time complaint not
a gcc warning, which is more worrisome. It is triggered by an argv list
where we pass (argv - 1) to something expecting only to index it
starting from '1'. I'm not sure yet if it is glibc being picky or if we
violate that assumption somewhere. I'll post a followup in the thread.

-Peff

^ permalink raw reply

* Re: A bug or a feature (git diff --author + --grep)
From: Björn Steinbrink @ 2009-10-04 13:19 UTC (permalink / raw)
  To: Pascal Obry; +Cc: git list
In-Reply-To: <4AC87837.2010004@obry.net>

On 2009.10.04 12:25:59 +0200, Pascal Obry wrote:
> 
> I would have expected the --author and --grep option to work as
> pipeline filters. The first on the command line being applied first.
> 
> In other word I would have expected:
> 
>    --author AND --grep
> 
> But it turn out that the result are maching either --author or --grep:
> 
>    --author OR --grep
> 
> For example on the Git project:
> 
>    $ git log --author=obry --grep=Cygwin
> 
> Returns commit from obry and commit having Cygwin in their log.
> 
> Is this a bug or a feature?

Feature. There's --all-match to switch on AND-mode.

Björn

^ permalink raw reply

* Re: [PATCH] tests: make all test files executable
From: Mark Rada @ 2009-10-04 13:18 UTC (permalink / raw)
  To: Jeff King; +Cc: Mark A Rada, git
In-Reply-To: <20091002083946.GA8627@coredump.intra.peff.net>

On 2009-10-02, at 4:39 AM, Jeff King wrote:

> On Fri, Oct 02, 2009 at 04:01:34AM -0400, Jeff King wrote:
>
>>> 0 files changed, 0 insertions(+), 0 deletions(-)
>>> mode change 100644 => 100755 t/t5531-deep-submodule-push.sh
>>> mode change 100644 => 100755 t/t9501-gitweb-standalone-http- 
>>> status.sh
>>>
>>> diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep- 
>>> submodule-push.sh
>>> old mode 100644
>>> new mode 100755
>>> diff --git a/t/t9501-gitweb-standalone-http-status.sh
>>> b/t/t9501-gitweb-standalone-http-status.sh
>>> old mode 100644
>>> new mode 100755
>>
>> When applying via "am", I only got the first change in my tree.  
>> I'll see
>> if I can confirm and make a test case.
>
> Ah, nevermind. The problem is that your patch was word-wrapped, making
> the second "diff --git" line bogus. It would have been nice to have it
> print a warning instead of silently ignoring that bit of the patch.
>

I didn't have format=flowed buggering things up this time, so I don't
quite understand the problem; could you please explain with more  
details?

When I try to apply the patch from a saved copy of the e-mail, I get
the following error:

	# git am ~/Downloads/\[PATCH\]\ tests_\ make\ all\ test\ files\  
executable.eml
	Patch format detection failed.
	zsh: exit 1     git am

The difference between the patch created by format-patch and the saved
e-mail is just some e-mail header information. Is that a different error
than what you were getting? I'm not sure what I'm doing wrong here, help
would be appreciated.

--
Mark Rada (ferrous26)
marada@uwaterloo.ca



^ permalink raw reply

* Re: stgit, rebasing with 100 patches
From: Jon Smirl @ 2009-10-04 13:00 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <9e4733910910011604w68cdca86l2baa2f2fe4db4a32@mail.gmail.com>

On Thu, Oct 1, 2009 at 7:04 PM, Jon Smirl <jonsmirl@gmail.com> wrote:
> I have 100 patches loaded into in stgit. My tree is at 2.6.30. Now I
> want to rebase to 2.6.31-rc1. About 30 of these hundred patches got
> committed in this interval.
>
> If I rebase directly to 2.6.31-rc1 I end up with a bunch of merge
> conflicts as the patches are applied. That's because patches 'a,b,c'
> got applied in the merge window. When I push 'a' back down it sees the
> combination of 'a,b,c' not just 'a'. It is unable to figure out that
> 'a' was applied and then 'b' and 'c' applied on top of it.
>
> Is there a better way to locate the patches the got applied?

A solution to this is to make an option on rebase that walks the patch
stack forward one commit at a time.

What does the --merged option do on stg rebase? The doc is rather sparse.


-- 
Jon Smirl
jonsmirl@gmail.com

^ permalink raw reply

* reset doesn't reset a revert?
From: Octavian Râşniţă @ 2009-10-04 12:18 UTC (permalink / raw)
  To: git

Hi,

Under Windows XP, using git version 1.6.4.msysgit.0, I have tried:

E:\lucru\git\k>echo foo > file.txt
E:\lucru\git\k>echo bar >> file.txt
E:\lucru\git\k>git init
Initialized empty Git repository in E:/lucru/git/k/.git/
E:\lucru\git\k>git add .
warning: CRLF will be replaced by LF in file.txt.
E:\lucru\git\k>git commit -a
[master (root-commit) e969cd5] First commit
warning: CRLF will be replaced by LF in file.txt.
1 files changed, 2 insertions(+), 0 deletions(-)
create mode 100644 file.txt
E:\lucru\git\k>echo baz >> file.txt
E:\lucru\git\k>git commit -a
warning: CRLF will be replaced by LF in file.txt.
warning: CRLF will be replaced by LF in file.txt.
[master warning: CRLF will be replaced by LF in file.txt.
fabd2f2] Added baz to file.txt
1 files changed, 1 insertions(+), 0 deletions(-)
E:\lucru\git\k>type file.txt
foo
bar
baz

#Until here, everything's OK.

E:\lucru\git\k>git  revert HEAD~
fatal: Cannot revert a root commit

#Does anyone know what does this mean? So I've tried with HEAD^ instead.

E:\lucru\git\k>git  revert HEAD^
More?
More?
Finished one revert.
[master 1beba20] Revert "Added baz to file.txt"
1 files changed, 0 insertions(+), 1 deletions(-)

# What should I respond to the questions More?
#I've seen that no matter what I type, it adds to the "HEAD" and tells that 
that commit can't be found, so I just pressed enter.

E:\lucru\git\k>git status
# On branch master
nothing to commit (working directory clean)
E:\lucru\git\k>git log --pretty=format:"%s %h"
WARNING: terminal is not fully functional
Revert "Added baz to file.txt" 1beba20
Added baz to file.txt fabd2f2
First commit e969cd5
(END)

#It seems that the revert commit was added successfully.

E:\lucru\git\k>type file.txt
foo
bar

#And it seems that not only the repository was changed, but the working 
directory also. Is it correct?

#Well, now let's say I discovered that this new commit was an error and I 
want to reset it.
#And I used HEAD^ because HEAD~ didn't work with revert.

E:\lucru\git\k>git reset HEAD^
More?
More?
E:\lucru\git\k>git log --pretty=format:"%s %h"
WARNING: terminal is not fully functional
Revert "Added baz to file.txt" 1beba20
Added baz to file.txt fabd2f2
First commit e969cd5
(END)

#Well, git reset didn't reset the latest commit.
#Does anyone know why or what I am doing wrong?

E:\lucru\git\k>git status
# On branch master
nothing to commit (working directory clean)
E:\lucru\git\k>git reset HEAD~
file.txt: locally modified
E:\lucru\git\k>git log --pretty=format:"%s %h"
WARNING: terminal is not fully functional
Added baz to file.txt fabd2f2
First commit e969cd5
(END)

#This time git reset resetted the latest HEAD.
#It seems that git reset wants the HEAD~ commit, while git revert wants the 
HEAD^ commit. Do you know why (or can I find an explanation for this 
somewhere)?

E:\lucru\git\k>type file.txt
foo
bar

#However, git reset modified just the repository and not the working 
directory.

I added the line baz in the file file.txt, commited this change and then 
reverted to the previous commit. This has also deleted the line "baz" from 
the file.
Then I resetted the last commit (the revert), however the line "baz" didn't 
appear in the file.

Is this something normal I should expect, or I am doing something wrong?

Thank you very much.

Octavian


^ permalink raw reply

* Alles wurde Git
From: Johannes Schindelin @ 2009-10-04 12:09 UTC (permalink / raw)
  To: git, msysgit

Hi,

we had much fun yesterday!  Just a few quotes so you know what I mean:

"Mac's your mistress, Windows' your wife" (Michael)

"I'm your user base" (Thorsten)

"We have 3 maintainers working on one master branch, and it is...
working" (Steffen)

[inappropriate joke censored]

"Yes, it is possible. But it will be hard." (Sverre)

"I am the ambassador of the ugly and the stupid" (Michael)

"Michael talked about what Subversion can do that Git cannot" -- "And I 
wasn't even hurt!"


Wrap-up:

We were kindly offered hosting by the Zuse-Institut Berlin (thanks 
Steffen) and there was always a ready supply of coffee and snacks, as well 
as a pretty fast internet connection.

After a round of introductions (apart from this developer, there were 
Michael Haggerty, Jens Lehmann, Heiko Voigt, Thorsten (I did not catch the 
surname), Sebastian Schuberth, Steffen Prohaska, Christian Halstrick and 
Sverre Rabbelier; Gitzilla and his friend Kristen joined us in the late 
afternoon), I started out with a few (hopefully interesting) tidbits and 
trivia about msysGit.  I also discussed the relationship between Cygwin, 
MSys, MinGW32 and msysGit, as well as why git-svn is so slow on Windows 
(and what one can do about it).

Then Michael talked a little bit about the versioning and the branching as 
well as common merging techniques in CVS, and how Subversion handles 
similar scenarios.  I was stunned to hear that Subversion allows you to 
cherry-pick commits, and even parts of a commit, from other branches, and 
keep track of it (i.e. remember at a later merge that this was already 
merged, and take that into account).  This is definitely something Git 
cannot do, and I can see that people could miss this feature after a 
SVN->Git switch.  I was also suprised to learn that Subversion allows you 
to edit commit messages and even authorship after the fact, but not 
patches.

Sverre went on to describe his work on hg-git and his plans to integrate 
it as a foreign vcs, which was when we had quite a few words on the 
current state of affairs of the foreign vcs support.  You all know what I 
mean.  We also discussed the possibility of git-remote-svn, which was seen 
as the natural way to integrate Subversion repositories as proper Git 
submodules... _iff_ we will be able to specify the clone URL together with 
the type as one line (i.e. optimizing for the common case instead of the 
obscure one).  One important outcome of that discussion was that we 
_cannot_ imitate git-svn there, as the commit messages and authorships can 
be edited (see above), so we would need to record the commits with a 
standard author and only the revision number as commit message, and put 
the mutable information into notes.  Git-svn can get away with the way it 
does things because it is meant to be single-user only, but with 
remote-svn submodules, you have to make sure that _every_ user will get 
the same SHA-1s regardless of when they happened to clone the Subversion 
repository.

Then Thorsten, who described himself as "probably the only person in the 
room without commits in git.git, so I guess I'm your user base" let us 
have some glimpses of real-world problems with Git.  One thing that was 
pretty obvious after thinking about it is the naming of branches.  Git is 
way too liberal, e.g. it allows "git checkout -b origin/master", when 
clearly "-t" was meant (and a look around told me that _everybody_ had 
this problem at some stage).  Unfortunately, it did not occur to us to 
discuss what can be done about these issues, let alone to make a list of 
them.

Jens offered some inside-stories of a company switching from RCS(!) to 
Git.  He also described himself and Heiko as heavy users of submodules, 
and took some time to explain in how many ways submodules are the 
neglected ugly duckling of Git.  He also showed us some of his 
work-in-progress to do something about it.  Another part of the 
presentation was an in-depth account how important msysGit was to the 
adoption of Git in the company.

Christian, who is working for SAP, and recently got involved in the 
eGit/JGit story, related a few interesting facts about the company he 
works for.  Already in 2001, they had a strong need for distributed 
version control, and came up with their own tool.  When they realized just 
how far Git advanced, a rather surprising change in policy allowed him not 
only to look into eGit/JGit, but also to work on it.  Which he did later 
that day, too, finding a bug in my diff implementation ;-)

Steffen and Sebastian then told us about another example of Git in a 
corporate setting.  What some of the Open Source guys did not appreciate 
fully before that presentation was that some things that cannot happen in 
Open Source are unavoidable in a company.  For example, you will always 
find people who work actively against having their patches reviewed.  
Another example is that accountability, and therefore code ownership, can 
be really necessary.  Steffen also described some of the issues with 
working on a large number of topic branches that need to be 
integrated/cleaned up/rebased:  Git lacks good tools for working 
collaboratively with topic branches that need to be rebased frequently.  
He also showed a rather complicated script that is necessary to coordinate 
work between 3 different maintainers (at 3 different locations).

We also got some hacking done, e.g. Heiko, Sebastian and me on 
Git-Cheetah, and of course the utterly useful "mispel" patch (that has not 
made it into git.git yet, much to my chagrin).  But most of the time we 
just had a good time and talked about what we love.

Oh, and Gitzilla entertained the whole party by recounting a few of the 
most hilarious emails authored by yours truly.

The unanimous consensus at the end of the day, after we were for dinner, 
was that we should do this more often: Git together.

Ciao,
Dscho




^ permalink raw reply

* Re: [PATCH] Add the utterly important 'mispel' command
From: Johannes Schindelin @ 2009-10-04 10:52 UTC (permalink / raw)
  To: Jeff King; +Cc: git, spearce, gitster
In-Reply-To: <20091004065239.GA7890@coredump.intra.peff.net>

Hi,

On Sun, 4 Oct 2009, Jeff King wrote:

> On Sun, Oct 04, 2009 at 12:41:55AM +0200, Johannes Schindelin wrote:
> 
> > --- a/builtin.h
> > +++ b/builtin.h
> > @@ -12,6 +12,7 @@ extern const char git_more_info_string[];
> >  
> >  extern void list_common_cmds_help(void);
> >  extern const char *help_unknown_cmd(const char *cmd);
> > +const char *help_mispeld_comd(const char *cmd);
> >  extern void prune_packed_objects(int);
> >  extern int read_line_with_nul(char *buf, int size, FILE *file);
> >  extern int fmt_merge_msg(int merge_summary, struct strbuf *in,
> 
> Hmph. This hunk doesn't apply to Shawn's master, and your blob sha1 is
> not in my repository for a 3-way merge. ;P

Hmm.  It might be based on 4msysgit.git's 'devel' branch.

> Also, I think there is a bug:
> 
>   $ git mispel show-branc
>   error: You probably meant show-index
> 
> Or is it intentional for it to throw the user off track?
> 
> Let this be a lesson, kids: don't drink and code.

Well, the command is called "mispel", not "autocorrect".  So I think you 
misunderstood the purpose of the patch.

Let this be a lesson, kids: don't review GitTogether patches before you 
had a drink.

Ciao,
Dscho


^ permalink raw reply

* A bug or a feature (git diff --author + --grep)
From: Pascal Obry @ 2009-10-04 10:25 UTC (permalink / raw)
  To: git list


I would have expected the --author and --grep option to work as pipeline 
filters. The first on the command line being applied first.

In other word I would have expected:

    --author AND --grep

But it turn out that the result are maching either --author or --grep:

    --author OR --grep

For example on the Git project:

    $ git log --author=obry --grep=Cygwin

Returns commit from obry and commit having Cygwin in their log.

Is this a bug or a feature?

Thanks,
Pascal.

-- 

--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|    http://www.obry.net  -  http://v2p.fr.eu.org
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver keys.gnupg.net --recv-key F949BD3B


^ permalink raw reply

* Re: Interim maintainer tree
From: Junio C Hamano @ 2009-10-04  9:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20091004064129.GB7076@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> For my stuff, I think you can just take over my tips. I was just
> collecting and polishing topics, while Shawn was doing integration. For
> topics I have, please use my versions rather than applying from the
> list; many of them have extra fixes squashed in, acked-by's collected
> from the list, etc.

Many topics Shawn merged to his 'pu' were identical to what you have,
except for mr/gitweb-snapshot.  The patch-ids in the series are the same
but you have Jakub's Ack added, so I'll pick yours up and rebuild.

> Here's a brief status report on what's in my tree:
>
>   proposed-master
>     One-off patches that I think should go straight to master.

I briefly looked at them after looking at what Shawn queued to his
'master'; they all looked sane.

>   ef/mscv-noreturn
>     This is the latest round and I think should be ready for at least
>     'next' (maybe even 'master' as it is really about the build and not
>     about functionality).
>
>   ef/msys-imap
>     This is from an RFC which has generated some comments. He should be
>     posting another round soon. 'pu' at best.
>
>   fc/mutt-alias
>     Latest round that addressed comments. Ready for 'next' if not
>     'master'.
>
>   jn/gitweb-patch
>     After some comments with Jakub, I think the code is right but he
>     promised a re-roll with more in the commit message.
>
>   mr/gitweb-snapshot
>     This is probably the pu rewind you saw. He posted a v5 of his
>     series. I didn't look at it closely, but Jakub ack'd it.
>
>   tf/doc-pt-br
>     Minor translation update, ack'd by somebody who can read it. :)
>     Ready for 'master'.

I've queued all of these to 'pu' for tonight as I haven't still fully
recovered yet from jetlag, but I agree with the above assessment.

> There are a few patches floating on the list that I haven't picked up or
> looked too closely at yet. Just so we don't drop anything, they are:
>
>   - curl http auth tweak:
>     http://article.gmane.org/gmane.comp.version-control.git/129455
>     The author said he didn't really test it, and I haven't set up http
>     auth to test it with, but probably somebody should do so before
>     applying. :)

Heh ;-)

>   - a new rev-cache from Nick

What Shawn had in his 'pu' had one patch removed from what I had earlier
(perhaps my 'tip list' was faulty).  I'll pick the new one up later.

>   - silence gcc warning:
>     http://article.gmane.org/gmane.comp.version-control.git/129485
>     The warning is overly cautious, I think, but it is a dubious enough
>     construct that it is probably worth fixing.

I cannot reach gmane now, but if this is about -Wextra, I'd rather not
touch it before the release.  "comparison between signed and unsigned"
tends to be excessive and IMNSHO it is crazy to use -Wextra and -Werror 
together.

>   - enable openssl on msvc
>     http://article.gmane.org/gmane.comp.version-control.msysgit/7238
>     This goes on top of ef/msys-imap, but I think that will be getting a
>     re-roll.

Ok.

Thanks.


^ permalink raw reply

* Re: previous references
From: Johan Herland @ 2009-10-04  9:27 UTC (permalink / raw)
  To: Octavian Râşniţă; +Cc: git
In-Reply-To: <8E72DAAF9F8E4024BB819F3F83CCFC79@teddy>

On Sunday 04 October 2009, Octavian Râşniţă wrote:
> Are the following commands specifying the same reference?
> 
> prompt> git log -1 HEAD^^^ ... log entry ...
> prompt> git log -1 HEAD^~2 ... log entry ...
> prompt> git log -1 HEAD~1^^ ... log entry ...
> prompt> git log -1 HEAD~3 ... log entry ...

Yes

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* previous references
From: Octavian Râşniţă @ 2009-10-04  8:31 UTC (permalink / raw)
  To: git

Hi,

Are the following commands specifying the same reference?

prompt> git log -1 HEAD^^^ ... log entry ... 
prompt> git log -1 HEAD^~2 ... log entry ... 
prompt> git log -1 HEAD~1^^ ... log entry ... 
prompt> git log -1 HEAD~3 ... log entry ... 

Thank you.

Octavian


^ permalink raw reply

* Re: "Not currently on any branch"
From: Clemens Buchacher @ 2009-10-04  7:22 UTC (permalink / raw)
  To: Tim; +Cc: git
In-Reply-To: <loom.20091002T215942-663@post.gmane.org>

On Fri, Oct 02, 2009 at 08:08:52PM +0000, Tim wrote:
> I have some code in a git repo that is "Not currently on any branch". Now,
> there's the master branch and another branch 'ui-integration' that I'm
> using in this project. I don't know how the project got into this headless
> state, but I need to be using the 'ui-integration' branch. 

It can happen either by explicitly detaching HEAD using "git checkout
<commit>", or if you used rebase and it is still in progress.

Clemens

^ permalink raw reply

* Re: [PATCH] Add the utterly important 'mispel' command
From: Jeff King @ 2009-10-04  6:52 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, spearce, gitster
In-Reply-To: <alpine.DEB.1.00.0910040040180.4985@pacific.mpi-cbg.de>

On Sun, Oct 04, 2009 at 12:41:55AM +0200, Johannes Schindelin wrote:

> --- a/builtin.h
> +++ b/builtin.h
> @@ -12,6 +12,7 @@ extern const char git_more_info_string[];
>  
>  extern void list_common_cmds_help(void);
>  extern const char *help_unknown_cmd(const char *cmd);
> +const char *help_mispeld_comd(const char *cmd);
>  extern void prune_packed_objects(int);
>  extern int read_line_with_nul(char *buf, int size, FILE *file);
>  extern int fmt_merge_msg(int merge_summary, struct strbuf *in,

Hmph. This hunk doesn't apply to Shawn's master, and your blob sha1 is
not in my repository for a 3-way merge. ;P

Also, I think there is a bug:

  $ git mispel show-branc
  error: You probably meant show-index

Or is it intentional for it to throw the user off track?

Let this be a lesson, kids: don't drink and code.

-Peff

^ permalink raw reply

* Re: Interim maintainer tree
From: Jeff King @ 2009-10-04  6:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7viqevu1zt.fsf@alter.siamese.dyndns.org>

On Sat, Oct 03, 2009 at 11:06:30PM -0700, Junio C Hamano wrote:

> Thanks, both.
> 
> I've fetched, but haven't fully examined "log ..spearce/*" nor "log ..peff/*"
> yet.
> 
> I noticed that some topics in 'pu' have been rebased (not complaining, but
> just making sure I am not hallucinating).
> 
> Do you have preferences/suggestions as to how to proceed?  Should I just
> take the tips over at this point, or do you have some more patches you
> were polishing but haven't pushed out that I should wait for?

For my stuff, I think you can just take over my tips. I was just
collecting and polishing topics, while Shawn was doing integration. For
topics I have, please use my versions rather than applying from the
list; many of them have extra fixes squashed in, acked-by's collected
from the list, etc.

Here's a brief status report on what's in my tree:

  proposed-master
    One-off patches that I think should go straight to master.

  ef/mscv-noreturn
    This is the latest round and I think should be ready for at least
    'next' (maybe even 'master' as it is really about the build and not
    about functionality).

  ef/msys-imap
    This is from an RFC which has generated some comments. He should be
    posting another round soon. 'pu' at best.

  fc/mutt-alias
    Latest round that addressed comments. Ready for 'next' if not
    'master'.

  jn/gitweb-patch
    After some comments with Jakub, I think the code is right but he
    promised a re-roll with more in the commit message.

  mr/gitweb-snapshot
    This is probably the pu rewind you saw. He posted a v5 of his
    series. I didn't look at it closely, but Jakub ack'd it.

  tf/doc-pt-br
    Minor translation update, ack'd by somebody who can read it. :)
    Ready for 'master'.

There are a few patches floating on the list that I haven't picked up or
looked too closely at yet. Just so we don't drop anything, they are:

  - curl http auth tweak:
    http://article.gmane.org/gmane.comp.version-control.git/129455
    The author said he didn't really test it, and I haven't set up http
    auth to test it with, but probably somebody should do so before
    applying. :)

  - a new rev-cache from Nick

  - silence gcc warning:
    http://article.gmane.org/gmane.comp.version-control.git/129485
    The warning is overly cautious, I think, but it is a dubious enough
    construct that it is probably worth fixing.

  - enable openssl on msvc
    http://article.gmane.org/gmane.comp.version-control.msysgit/7238
    This goes on top of ef/msys-imap, but I think that will be getting a
    re-roll.

-Peff

^ permalink raw reply

* Re: Interim maintainer tree
From: Junio C Hamano @ 2009-10-04  6:06 UTC (permalink / raw)
  To: Shawn O. Pearce, Jeff King; +Cc: git
In-Reply-To: <20090925160504.GW14660@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Junio is on vaction for the next week.  In his absence Peff and I
> are trying to keep up with current patches in my fork:
>
>   git://repo.or.cz/git/spearce.git
>   http://repo.or.cz/r/git/spearce.git
>
> Right now the tree matches Junio's last push, I'll try to pick up
> the patches since then and push later today.

Thanks, both.

I've fetched, but haven't fully examined "log ..spearce/*" nor "log ..peff/*"
yet.

I noticed that some topics in 'pu' have been rebased (not complaining, but
just making sure I am not hallucinating).

Do you have preferences/suggestions as to how to proceed?  Should I just
take the tips over at this point, or do you have some more patches you
were polishing but haven't pushed out that I should wait for?

^ permalink raw reply

* [PATCH] Add the utterly important 'mispel' command
From: Johannes Schindelin @ 2009-10-03 22:41 UTC (permalink / raw)
  To: git; +Cc: spearce, peff, gitster


If you do not remember how to mispel a command, you need some help.

Provide it.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: A Large Angry SCM <gitzilla@gmail.com>
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Christian Halstrick <christian.halstrick@sap.com>
Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Acked-By: <paisleyklm@gmail.com>
---

	Please apply.

 Makefile         |    1 +
 builtin-mispel.c |   11 +++++++++++
 builtin.h        |    2 ++
 git.c            |    1 +
 help.c           |   31 +++++++++++++++++++++++++++++++
 5 files changed, 46 insertions(+), 0 deletions(-)
 create mode 100644 builtin-mispel.c

diff --git a/Makefile b/Makefile
index 690ac55..0b48b3b 100644
--- a/Makefile
+++ b/Makefile
@@ -610,6 +610,7 @@ BUILTIN_OBJS += builtin-merge-file.o
 BUILTIN_OBJS += builtin-merge-ours.o
 BUILTIN_OBJS += builtin-merge-recursive.o
 BUILTIN_OBJS += builtin-mktree.o
+BUILTIN_OBJS += builtin-mispel.o
 BUILTIN_OBJS += builtin-mv.o
 BUILTIN_OBJS += builtin-name-rev.o
 BUILTIN_OBJS += builtin-pack-objects.o
diff --git a/builtin-mispel.c b/builtin-mispel.c
new file mode 100644
index 0000000..e685f91
--- /dev/null
+++ b/builtin-mispel.c
@@ -0,0 +1,11 @@
+#include "cache.h"
+#include "builtin.h"
+
+int cmd_mispel(int argc, const char **argv, const char *prefix)
+{
+	if (argc < 2)
+		die ("What command do you want to mispel?");
+	error("You probably meant %s", help_mispeld_comd(argv[1]));
+	return 0;
+
+}
diff --git a/builtin.h b/builtin.h
index 20427d2..2973d90 100644
--- a/builtin.h
+++ b/builtin.h
@@ -12,6 +12,7 @@ extern const char git_more_info_string[];
 
 extern void list_common_cmds_help(void);
 extern const char *help_unknown_cmd(const char *cmd);
+const char *help_mispeld_comd(const char *cmd);
 extern void prune_packed_objects(int);
 extern int read_line_with_nul(char *buf, int size, FILE *file);
 extern int fmt_merge_msg(int merge_summary, struct strbuf *in,
@@ -73,6 +74,7 @@ extern int cmd_merge_ours(int argc, const char **argv, const char *prefix);
 extern int cmd_merge_file(int argc, const char **argv, const char *prefix);
 extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);
 extern int cmd_mktree(int argc, const char **argv, const char *prefix);
+extern int cmd_mispel(int argc, const char **argv, const char *prefix);
 extern int cmd_mv(int argc, const char **argv, const char *prefix);
 extern int cmd_name_rev(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);
diff --git a/git.c b/git.c
index 807d875..2caca54 100644
--- a/git.c
+++ b/git.c
@@ -327,6 +327,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
 		{ "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
 		{ "mktree", cmd_mktree, RUN_SETUP },
+		{ "mispel", cmd_mispel },
 		{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 		{ "name-rev", cmd_name_rev, RUN_SETUP },
 		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
diff --git a/help.c b/help.c
index 6c46d8b..97f0f22 100644
--- a/help.c
+++ b/help.c
@@ -296,6 +296,37 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
 	old->names = NULL;
 }
 
+const char *help_mispeld_comd(const char *cmd)
+{
+	struct cmdnames main_cmds, other_cmds;
+	int i;
+
+	memset(&main_cmds, 0, sizeof(main_cmds));
+	memset(&other_cmds, 0, sizeof(main_cmds));
+	git_config(git_unknown_cmd_config, NULL);
+
+	load_command_list("git-", &main_cmds, &other_cmds);
+
+	add_cmd_list(&main_cmds, &aliases);
+	add_cmd_list(&main_cmds, &other_cmds);
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(main_cmds.names), cmdname_compare);
+	uniq(&main_cmds);
+
+	/* This reuses cmdname->len for similarity index */
+	for (i = 0; i < main_cmds.cnt; ++i)
+		main_cmds.names[i]->len =
+			levenshtein(cmd, main_cmds.names[i]->name, 0, 2, 1, 4);
+
+	qsort(main_cmds.names, main_cmds.cnt,
+	      sizeof(*main_cmds.names), levenshtein_compare);
+
+	if (main_cmds.cnt< 2)
+		die ("Uh oh. Your system reports no Git commands at all.");
+
+	return main_cmds.names[1]->name;
+}
+
 const char *help_unknown_cmd(const char *cmd)
 {
 	int i, n, best_similarity = 0;
-- 
1.6.4.msysgit.0.1.g2dcf.dirty

^ permalink raw reply related

* Re: [PATCH/RFC 1/7] imap-send: use separate read and write fds
From: Johannes Sixt @ 2009-10-03 21:34 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, Jeff King, git
In-Reply-To: <40aa078e0910031144r735a6fdq25efc1e57a1d4c33@mail.gmail.com>


On Samstag, 3. Oktober 2009, Erik Faye-Lund wrote:
> - It needs some additional patching to get tunnelling support working
> on Windows, because we can't exec "/bin/sh" there. Changing it to
> "c:\\msysgit\\bin\\sh.exe" makes tunneling work for me, but isn't
> exactly portable across installations.

It should be fine to just exec "sh" (without a path).

-- Hannes

^ permalink raw reply

* Re: [PATCH/RFC 5/7] imap-send: provide fall-back random-source
From: Jeff King @ 2009-10-03 20:52 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git, mike
In-Reply-To: <20091003204317.GB9058@sigill.intra.peff.net>

On Sat, Oct 03, 2009 at 04:43:17PM -0400, Jeff King wrote:

> The other confusing bit is that the code carefully tracks the "uid"
> (deep within the call chain it munges cb.ctx, which is a pointer to uid)
> which is assigned to the newly created message by the server. This could
> be used by a client to later refer to the same message unambiguously.
> But we never do that, and just throw away the uid value that the server
> gives us.  Again, I suspect this is a holdover from isync wanting to do
> repeated synchronization (and it looks like this x-tuid stuff may be
> about working around servers which don't support certain uid
> operations).
> 
> So that could probably be ripped out, too, with no ill effect.

And here is a patch (on top of the earlier one) to do that.

Even more can be ripped out from the lower levels, too, I'm not sure if
it is worth it. Ripping out the arc4 code is worthwhile, because it
solves a portability problem. Ripping out more isn't really helping us
much. Less code makes it easier to read, but given our lack of tests and
my relatively small knowledge of this code, it is entirely possible I am
introducing new bugs.

---

 imap-send.c |   25 ++++++-------------------
 1 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index d60a0bd..8da7a94 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1149,7 +1149,7 @@ static int imap_make_flags(int flags, char *buf)
 	return d;
 }
 
-static int imap_store_msg(struct store *gctx, struct msg_data *data, int *uid)
+static int imap_store_msg(struct store *gctx, struct msg_data *data)
 {
 	struct imap_store *ctx = (struct imap_store *)gctx;
 	struct imap *imap = ctx->imap;
@@ -1171,26 +1171,14 @@ static int imap_store_msg(struct store *gctx, struct msg_data *data, int *uid)
 	}
 	flagstr[d] = 0;
 
-	if (!uid) {
-		box = gctx->conf->trash;
-		prefix = ctx->prefix;
-		cb.create = 1;
-		if (ctx->trashnc)
-			imap->caps = imap->rcaps & ~(1 << LITERALPLUS);
-	} else {
-		box = gctx->name;
-		prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
-		cb.create = 0;
-	}
-	cb.ctx = uid;
+	box = gctx->name;
+	prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
+	cb.create = 0;
 	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" %s", prefix, box, flagstr);
 	imap->caps = imap->rcaps;
 	if (ret != DRV_OK)
 		return ret;
-	if (!uid)
-		ctx->trashnc = 0;
-	else
-		gctx->count++;
+	gctx->count++;
 
 	return DRV_OK;
 }
@@ -1366,7 +1354,6 @@ int main(int argc, char **argv)
 {
 	struct msg_data all_msgs, msg;
 	struct store *ctx = NULL;
-	int uid = 0;
 	int ofs = 0;
 	int r;
 	int total, n = 0;
@@ -1420,7 +1407,7 @@ int main(int argc, char **argv)
 			break;
 		if (server.use_html)
 			wrap_in_html(&msg);
-		r = imap_store_msg(ctx, &msg, &uid);
+		r = imap_store_msg(ctx, &msg);
 		if (r != DRV_OK)
 			break;
 		n++;

^ permalink raw reply related

* Re: [PATCH/RFC 5/7] imap-send: provide fall-back random-source
From: Jeff King @ 2009-10-03 20:43 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git, mike
In-Reply-To: <40aa078e0910031145l2849697ftd2da2f5aaa28d957@mail.gmail.com>

On Sat, Oct 03, 2009 at 11:45:55AM -0700, Erik Faye-Lund wrote:

> I tried to trace this a little bit, but I got lost a bit in the
> callback-stuff. However, it looks to me like it might get sent to the
> server: it gets injected into cb.data in imap_store_msg(), and in
> v_issue_imap_cmd() it gets sent to the server if the LITERALPLUS
> capability is supported. I might be wrong though, as I find this code
> quite confusing.

It does get stored on the server either way (LITERALPLUS is just an imap
server extension that gives us different options for how we send). The
code actually munges an extra "X-TUID" rfc822 header into your message,
which has a randomly generated value. But we never _use_ the value for
anything. I think it is just inherited via cut-and-paste from the
original isync, which I guess actually does use it for synchronization.

The patch below rips it (and the arc4 code) out completely. It still
works in my simple test case, but I am not really an imap-send user, so
caveat emptor.

The other confusing bit is that the code carefully tracks the "uid"
(deep within the call chain it munges cb.ctx, which is a pointer to uid)
which is assigned to the newly created message by the server. This could
be used by a client to later refer to the same message unambiguously.
But we never do that, and just throw away the uid value that the server
gives us.  Again, I suspect this is a holdover from isync wanting to do
repeated synchronization (and it looks like this x-tuid stuff may be
about working around servers which don't support certain uid
operations).

So that could probably be ripped out, too, with no ill effect.

That's just from looking at the code for a few minutes. I would not be
surprised if there is more useless cruft, nor would I be surprised to
find that I am totally wrong about something above.

Anyway, here is the patch to rip out the arc4 stuff. It has a very
pleasant diff-stat. :)

---
 imap-send.c |  130 ++--------------------------------------------------------
 1 files changed, 5 insertions(+), 125 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 3847fd1..d60a0bd 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -123,9 +123,6 @@ static int nfvasprintf(char **strp, const char *fmt, va_list ap)
 	return len;
 }
 
-static void arc4_init(void);
-static unsigned char arc4_getbyte(void);
-
 struct imap_server_conf {
 	char *name;
 	char *tunnel;
@@ -489,52 +486,6 @@ static int nfsnprintf(char *buf, int blen, const char *fmt, ...)
 	return ret;
 }
 
-static struct {
-	unsigned char i, j, s[256];
-} rs;
-
-static void arc4_init(void)
-{
-	int i, fd;
-	unsigned char j, si, dat[128];
-
-	if ((fd = open("/dev/urandom", O_RDONLY)) < 0 && (fd = open("/dev/random", O_RDONLY)) < 0) {
-		fprintf(stderr, "Fatal: no random number source available.\n");
-		exit(3);
-	}
-	if (read_in_full(fd, dat, 128) != 128) {
-		fprintf(stderr, "Fatal: cannot read random number source.\n");
-		exit(3);
-	}
-	close(fd);
-
-	for (i = 0; i < 256; i++)
-		rs.s[i] = i;
-	for (i = j = 0; i < 256; i++) {
-		si = rs.s[i];
-		j += si + dat[i & 127];
-		rs.s[i] = rs.s[j];
-		rs.s[j] = si;
-	}
-	rs.i = rs.j = 0;
-
-	for (i = 0; i < 256; i++)
-		arc4_getbyte();
-}
-
-static unsigned char arc4_getbyte(void)
-{
-	unsigned char si, sj;
-
-	rs.i++;
-	si = rs.s[rs.i];
-	rs.j += si;
-	sj = rs.s[rs.j];
-	rs.s[rs.i] = sj;
-	rs.s[rs.j] = si;
-	return rs.s[(si + sj) & 0xff];
-}
-
 static struct imap_cmd *v_issue_imap_cmd(struct imap_store *ctx,
 					 struct imap_cmd_cb *cb,
 					 const char *fmt, va_list ap)
@@ -1198,88 +1149,20 @@ static int imap_make_flags(int flags, char *buf)
 	return d;
 }
 
-#define TUIDL 8
-
 static int imap_store_msg(struct store *gctx, struct msg_data *data, int *uid)
 {
 	struct imap_store *ctx = (struct imap_store *)gctx;
 	struct imap *imap = ctx->imap;
 	struct imap_cmd_cb cb;
-	char *fmap, *buf;
 	const char *prefix, *box;
-	int ret, i, j, d, len, extra, nocr;
-	int start, sbreak = 0, ebreak = 0;
-	char flagstr[128], tuid[TUIDL * 2 + 1];
+	int ret, d;
+	char flagstr[128];
 
 	memset(&cb, 0, sizeof(cb));
 
-	fmap = data->data;
-	len = data->len;
-	nocr = !data->crlf;
-	extra = 0, i = 0;
-	if (!CAP(UIDPLUS) && uid) {
-	nloop:
-		start = i;
-		while (i < len)
-			if (fmap[i++] == '\n') {
-				extra += nocr;
-				if (i - 2 + nocr == start) {
-					sbreak = ebreak = i - 2 + nocr;
-					goto mktid;
-				}
-				if (!memcmp(fmap + start, "X-TUID: ", 8)) {
-					extra -= (ebreak = i) - (sbreak = start) + nocr;
-					goto mktid;
-				}
-				goto nloop;
-			}
-		/* invalid message */
-		free(fmap);
-		return DRV_MSG_BAD;
-	mktid:
-		for (j = 0; j < TUIDL; j++)
-			sprintf(tuid + j * 2, "%02x", arc4_getbyte());
-		extra += 8 + TUIDL * 2 + 2;
-	}
-	if (nocr)
-		for (; i < len; i++)
-			if (fmap[i] == '\n')
-				extra++;
-
-	cb.dlen = len + extra;
-	buf = cb.data = xmalloc(cb.dlen);
-	i = 0;
-	if (!CAP(UIDPLUS) && uid) {
-		if (nocr) {
-			for (; i < sbreak; i++)
-				if (fmap[i] == '\n') {
-					*buf++ = '\r';
-					*buf++ = '\n';
-				} else
-					*buf++ = fmap[i];
-		} else {
-			memcpy(buf, fmap, sbreak);
-			buf += sbreak;
-		}
-		memcpy(buf, "X-TUID: ", 8);
-		buf += 8;
-		memcpy(buf, tuid, TUIDL * 2);
-		buf += TUIDL * 2;
-		*buf++ = '\r';
-		*buf++ = '\n';
-		i = ebreak;
-	}
-	if (nocr) {
-		for (; i < len; i++)
-			if (fmap[i] == '\n') {
-				*buf++ = '\r';
-				*buf++ = '\n';
-			} else
-				*buf++ = fmap[i];
-	} else
-		memcpy(buf, fmap + i, len - i);
-
-	free(fmap);
+	cb.dlen = data->len;
+	cb.data = xmalloc(cb.dlen);
+	memcpy(cb.data, data->data, data->len);
 
 	d = 0;
 	if (data->flags) {
@@ -1491,9 +1374,6 @@ int main(int argc, char **argv)
 
 	git_extract_argv0_path(argv[0]);
 
-	/* init the random number generator */
-	arc4_init();
-
 	setup_git_directory_gently(&nongit_ok);
 	git_config(git_imap_config, NULL);
 

^ permalink raw reply related

* Re: [PATCH/RFC 1/7] imap-send: use separate read and write fds
From: Jeff King @ 2009-10-03 20:34 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: msysgit, git
In-Reply-To: <40aa078e0910031144r735a6fdq25efc1e57a1d4c33@mail.gmail.com>

On Sat, Oct 03, 2009 at 11:44:50AM -0700, Erik Faye-Lund wrote:

> Yeah, this is about Windows portability.
> 
> I'll add something like "This is a patch that enables us to use the
> run-command API, which is supported on Windows." to the commit-message
> in the next round. Is that enough?

Yeah, that would be fine. I was just left scratching my head wondering
what subtle portability difference the two descriptors could have. But
if it really is just a cleanup for the next patch, that's OK; just say
so.

-Peff

^ permalink raw reply

* Re: MSVC build broken (on cygwin)
From: Marius Storm-Olsen @ 2009-10-03 20:29 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: Alex Riesen, GIT Mailing-list, Shawn O. Pearce
In-Reply-To: <4AC7AEB9.3030404@ramsay1.demon.co.uk>

Ramsay Jones said the following on 03.10.2009 22:06:
> Marius Storm-Olsen wrote:
>> So, something like this maybe, in git-compat-util.h:
>>
>> #if defined(__MINGW32__) || defined(_MSC_VER)
>> #  defined API_WIN32
>> #  defined OS_WINDOWS
>> #elif defined(__CYGWIN__)
>> #  defined API_POSIX
>> #  defined OS_WINDOWS
>> #else
>> #  defined API_POSIX
>> #endif
> 
> This is a much better idea.

OK, I'll write up a patch, tomorrow or Monday.

...
> So, I think something like this in git-compat-util.h:
> 
> #if defined(_WIN32) && !defined(__CYGWIN__)
> # define WIN32_API
> # define WIN32_LEAN_AND_MEAN
> # include <winsock2.h>
> # include <windows.h>
> #endif

I agree with this one. Send a patch, and I'll ack.


> and replace all #if(n)def WIN32|_WIN32 with #if(n)def WIN32_API.

Ok, I might look into that too then.


> The only use of the <windows.h> header by cygwin can be moved
> into compat/cygwin.c. (I don't much like cygwin using the
> Win32 API anyway!)

I don't have Cygwin installed, so I won't touch this one.


>> So, then we can use #ifdef API_WIN32 when using the Win32 API is the 
>> only option/preferred for MinGW or MSVC; and use #ifdef OS_WINDOWS 
>> when there are things that affect all the Windows builds.
>>
>> Opinions?
> 
> see above. I don't think OS_WINDOWS is necessary.

Well, it was mostly intended where we'd have code/algorithms which are 
platform specific, and not really compiler specific; such as the *stat() 
optimizations. They could probably be joined into an OS_WINDOWS section, 
with a POSIX_API hunk for the Cygwin fallbacks.

Not really important though. Hopefully there won't be too much platform 
specific stuff anyways.

--
.marius

^ permalink raw reply

* Re: [PATCH] MSVC: Enable OpenSSL, and translate -lcrypto
From: Erik Faye-Lund @ 2009-10-03 20:17 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: git, msysgit
In-Reply-To: <4AC7AF58.1090603@gmail.com>


On Sat, Oct 3, 2009 at 1:08 PM, Marius Storm-Olsen
<marius@storm-olsen.com> wrote:
>> Didn't my 7/7 already do this hunk?
>
> Not for MSVC. It has it's own section above the MinGW part. Besides, the
> -lcrypto 'translation' is critical for it to link.

Ahh, thanks for the clarification :)

-- 
Erik "kusma" Faye-Lund
kusmabite@gmail.com
(+47) 986 59 656

^ permalink raw reply

* Re: MSVC build broken (on cygwin)
From: Ramsay Jones @ 2009-10-03 19:36 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: GIT Mailing-list, Shawn O. Pearce
In-Reply-To: <4AC5B4AE.5070307@gmail.com>

Marius Storm-Olsen wrote:
> ...
> Clone the git://repo.or.cz/msvcgit.git, and run the 
> setup_32bit_env.cmd script in there, and you should have everything 
> you need to both compile and link Git with MSVC.
> 

Hmm, I'm trying to avoid YASORD (Yet Another Set Of Redundant
Dependencies ;-)  On my laptop, I currently have 4 zlib installations,
5 openssl installations, 3 iconv's ...

As I said earlier (see below), I'm not so interested in getting the
msvc build to work for me, rather than understand why it works for you,
since it should not work!

Having said that, I *may* try to get it working on my cygwin
installation. However, I'm much more likely to make some changes to
the build/Makefile to allow the dependent libraries to be installed
in different locations :) There is nothing wrong with my zlib at
C:\zlib.

>> Anyway, the point is *not* to get the msvc build to work for me; rather
>> it is to understand why the build *works* for you. ;-)
> 
> First of all, thanks for the thorough report! :)

You're welcome.

> Second, I just recompiled, and it magically works for me. Why is a 
> good question, since I also think it shouldn't at this point. The

Oh, you *really* don't want "magic" to be the answer... :P

> So, obviously, some magic in there is making it work for me. I have a 
> hard time locating the magic in question though. :-/

Which shell are you using? MSYS-bash?
Which make are you using? MSYS-GNU?
Which Perl are you using? ActiveState? MSYS?

I'm using cygwin 1.5.22, along with the cygwin versions of
bash 3.2, GNU make 3.81, perl 5.8.7

I noticed that the clink.pl script was not returning the correct exit
code to the Makefile, which is why I ended up snipping 940 lines of
output from the earlier #error demonstration; the Makefile does not
notice when the compile exits with an error.

In order to fix this issue for me, I made the following change:

-- >8 --
diff --git a/compat/vcbuild/scripts/clink.pl b/compat/vcbuild/scripts/clink.pl
index 0ffd59f..3e4e501 100644
--- a/compat/vcbuild/scripts/clink.pl
+++ b/compat/vcbuild/scripts/clink.pl
@@ -45,4 +45,18 @@ if ($is_linking) {
 	push(@args, @cflags);
 }
 #printf("**** @args\n");
-exit system(@args);
+
+system(@args);
+
+if ($? == -1) {
+	print "clink.pl: failed to execute: $!\n";
+	exit 1;
+}
+elsif ($? & 127) {
+	printf "clink.pl: child died with signal %d%s\n",
+		($? & 127), ($? & 128) ? ', coredump.' : '.';
+	exit 1;
+}
+
+exit $? >> 8;
+
-- >8 --

See "perldoc -f system" for more explanation of the above. This is
how it works on unix and unix-alike systems, so this may not work
too well on (say) ActiveState Perl; Dunno. Also, according to this
documentation, the form of the call to system() should result in a
call to an exec function, rather than using a shell; this may or
may not be true on other platforms.

Having fixed that problem, I modified clink.pl again so that it
would run args.exe rather than cl.exe; this allowed me to see,
using: make -> perl -> "system()" -> args.exe, just what will be
passed to the compiler.

Just in case you can't guess, create args.exe from:

    $ cat -n args.c
         1	#include <stdio.h>
         2	
         3	int main(int argc, char *argv[])
         4	{
         5		int i;
         6	
         7		for (i=0; i< argc; i++) {
         8			printf("argv[%d] = '%s'\n", i, argv[i]);
         9		}
        10		exit(1);
        11	}
        12	
    $ 

and put it somewhere in your path (~/bin for me).

    $ make MSVC=1
    GIT_VERSION = 1.6.5.rc1.38.gb4f27.dirty
        * new build flags or prefix
        CC fast-import.o
    argv[0] = 'args'
    argv[1] = '-Fofast-import.o'
    argv[2] = '-c'
    argv[3] = '-nologo'
    argv[4] = 'fast-import.c'
    argv[5] = '-I.'
    argv[6] = '-I../zlib'
    argv[7] = '-Icompat/vcbuild'
    argv[8] = '-Icompat/vcbuild/include'
    argv[9] = '-DWIN32-D_CONSOLE'
    [...snipped...]
    argv[56] = '-Icompat/regex'
    make: *** [fast-import.o] Error 1

Perhaps you could try a similar exercise?

Hmm, do you have any funny environment variables set which msvc is
picking up? 
Oh, what about the CL variable?

> That being said, does adding the space between the defines fix the 
> MSVC compilation using Cygwin's GNU Make? It's none-the-less a correct 
> patch, so you get an ack from me. Thanks!
> 
> Acked-by: Marius Storm-Olsen <mstormo@gmail.com>
> 

Thanks!

ATB,
Ramsay Jones

^ permalink raw reply related

* Re: MSVC build broken (on cygwin)
From: Ramsay Jones @ 2009-10-03 20:06 UTC (permalink / raw)
  To: Marius Storm-Olsen; +Cc: Alex Riesen, GIT Mailing-list, Shawn O. Pearce
In-Reply-To: <4AC5BEA6.5000102@gmail.com>

Marius Storm-Olsen wrote:
> Apparently, nothing is broken in neither Cygwin, MinGW or MSVC after 
> Ramsays whitespace fix, but I'm sure it might get hairy later, if/when 
> we get more Windows contributions. Keeping the guards right could get 
> tricky.
> 

Right! Thus my earlier nervousness. :P

> So, something like this maybe, in git-compat-util.h:
> 
> #if defined(__MINGW32__) || defined(_MSC_VER)
> #  defined API_WIN32
> #  defined OS_WINDOWS
> #elif defined(__CYGWIN__)
> #  defined API_POSIX
> #  defined OS_WINDOWS
> #else
> #  defined API_POSIX
> #endif
> 

This is a much better idea.

Note that I also have Digital-Mars C/C++ 8.50, Open Watcom C/C++ 1.8
and lcc 4.2 installed. So, lets add to our previous tests:

Digital-Mars:

    $ dmc hello.c
    link hello,,,user32+kernel32/noi;

    $ ./hello.exe
    _WIN32
    WIN32
    Hello world
    $ 

    $ dmc -DIW_H hello.c
    link hello,,,user32+kernel32/noi;

    $ ./hello.exe
    _WIN32
    WIN32
    Hello world
    $ 

    $ dmc -DWIN32-D_CONSOLE hello.c
    Command line error: bad -D switch, need '=' after macro name--- errorlevel 1
    $ 

Open Watcom:

    $ wcl386 hello.c
    [...compiler output snipped...]
    $ ./hello.exe
    _WIN32
    Hello world
    $ 

    $ wcl386 -DIW_H hello.c
    [...compiler output snipped...]
    $ ./hello.exe
    _WIN32
    Hello world
    $ 

    $ wcl386 -DWIN32-D_CONSOLE hello.c
    Open Watcom C/C++32 Compile and Link Utility Version 1.8
    Portions Copyright (c) 1988-2002 Sybase, Inc. All Rights Reserved.
    Source code is available under the Sybase Open Watcom Public License.
    See http://www.openwatcom.org/ for details.
           wcc386 hello.c  -DWIN32 -D_CONSOLE
    Open Watcom C32 Optimizing Compiler Version 1.8
    Portions Copyright (c) 1984-2002 Sybase, Inc. All Rights Reserved.
    Source code is available under the Sybase Open Watcom Public License.
    See http://www.openwatcom.org/ for details.
    hello.c: 25 lines, included 757, 0 warnings, 0 errors
    Code size: 52
           wlink @__wcl__.lnk
    Open Watcom Linker Version 1.8
    Portions Copyright (c) 1985-2002 Sybase, Inc. All Rights Reserved.
    Source code is available under the Sybase Open Watcom Public License.
    See http://www.openwatcom.org/ for details.
    loading object files
    searching libraries
    creating a Windows NT character-mode executable
    $ ./hello.exe
    _WIN32
    WIN32
    Hello world
    $ 
[Note: I didn't snip the compiler output here so that you could see that
the Watcom driver program had "fixed" the malformed -Define and passed
it as two separate parameters to the compiler proper!]

Also note that Open Watcom is currently being ported to Linux, I *think*
Digital-Mars already has a Linux version and lcc does have a Linux
version. However, I think it's reasonably safe to assume we won't see a
Linux version of msvc.

So, I think something like this in git-compat-util.h:

#if defined(_WIN32) && !defined(__CYGWIN__)
# define WIN32_API
# define WIN32_LEAN_AND_MEAN
# include <winsock2.h>
# include <windows.h>
#endif

and replace all #if(n)def WIN32|_WIN32 with #if(n)def WIN32_API.

The only use of the <windows.h> header by cygwin can be moved
into compat/cygwin.c. (I don't much like cygwin using the
Win32 API anyway!)

> So, then we can use #ifdef API_WIN32 when using the Win32 API is the 
> only option/preferred for MinGW or MSVC; and use #ifdef OS_WINDOWS 
> when there are things that affect all the Windows builds.
> 
> Opinions?

see above. I don't think OS_WINDOWS is necessary.

Anyway, *something* like this would be an improvement.

ATB,
Ramsay Jones

^ permalink raw reply

* Re: [PATCH] MSVC: Enable OpenSSL, and translate -lcrypto
From: Marius Storm-Olsen @ 2009-10-03 20:08 UTC (permalink / raw)
  To: Erik Faye-Lund; +Cc: git, msysgit
In-Reply-To: <40aa078e0910031305u38cfaf4aua72d4c7af2a470b2@mail.gmail.com>


Erik Faye-Lund said the following on 03.10.2009 22:05:
> On Sat, Oct 3, 2009 at 1:00 PM, Marius Storm-Olsen <mstormo@gmail.com> wrote:
>>  This patch was actually sent using the MSVC
>>  git-imap-send.exe to my GMail account.
>>   D:\msvc\git>cat
>> 0001-MSVC-Enable-OpenSSL-and-translate-lcrypto.patch |
>> git-imap-send.exe
>>   Resolving imap.gmail.com... ok
>>   Connecting to 74.125.79.109:993... ok
>>   Logging in...
>>   sending 1 message
>>   100% (1/1) done
>>  :)
> 
> Awesome :)

Yup, only sad to see that GMail dropped the In-reply-to when sending :-/

>> diff --git a/Makefile b/Makefile
>> index 8818f0f..c4b91d8 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -881,7 +881,6 @@ ifdef MSVC
>>        GIT_VERSION := $(GIT_VERSION).MSVC
>>        pathsep = ;
>>        NO_PREAD = YesPlease
>> -       NO_OPENSSL = YesPlease
>>        NO_LIBGEN_H = YesPlease
>>        NO_SYMLINK_HEAD = YesPlease
>>        NO_IPV6 = YesPlease
> 
> Didn't my 7/7 already do this hunk?

Not for MSVC. It has it's own section above the MinGW part. Besides, the
-lcrypto 'translation' is critical for it to link.

--
.marius

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox