Re: [PATCH] config.mak.uname: Cygwin: Use renames for creation

2015-08-08 Thread Mark Levedahl

On 08/07/2015 04:30 PM, Adam Dinwoodie wrote:

When generating build options for Cygwin, enable
OBJECT_CREATION_USES_RENAMES.  This is necessary to use Git on Windows
shared directories, and is already enabled for the MinGW and plain
Windows builds.

This problem was reported on the Cygwin mailing list at
https://cygwin.com/ml/cygwin/2015-08/msg00102.html (amongst others) and
is being applied as a manual patch to the Cygwin builds until the patch
is taken here.

Reported-by: Peter Rosin p...@lysator.liu.se
Signed-off-by: Adam Dinwoodie a...@dinwoodie.org
---
  config.mak.uname | 1 +
  1 file changed, 1 insertion(+)

diff --git a/config.mak.uname b/config.mak.uname
index 943c439..be5cbec 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -187,6 +187,7 @@ ifeq ($(uname_O),Cygwin)
X = .exe
UNRELIABLE_FSTAT = UnfortunatelyYes
SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
+   OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
  endif
  ifeq ($(uname_S),FreeBSD)
NEEDS_LIBICONV = YesPlease
I've been supporting use of git on cygwin since about 2008, this issue 
has never risen that I know. Whatever issue is being patched around 
here, if truly repeatable, should be handled by the cygwin dll as that 
code is focused on providing full linux compatibility. If git on linux 
does need this patch, git on cygwin should not, either. So, I vote 
against this.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 23/23] checkout: retire --ignore-other-worktrees in favor of --force

2015-07-07 Thread Mark Levedahl

On 07/06/2015 03:40 PM, Junio C Hamano wrote:
If you are extending the history of some branch, then you would want 
to be on that branch. Why would you want to have another worktree that 
will get into a confusing state once you create that commit on the 
checked out branch in this newly created worktree? Wasn't the whole 
point of making the primary repository aware of the secondary 
worktrees via the linked checkout mechanism because that confusion 
was the biggest sore point of the old contrib/workdir implementation? 


The only issue I have with git-new-workdir is that git-gc in one 
worktree is unaware of what is in use in another so can prune things 
away. The linked worktrees here nicely solve that problem.


The main use I have of maintaining multiple checkouts of one branch is 
for testing / analysis (where said tests can take days to weeks to run). 
Disallowing use of git's normal mechanism of tracking what is checked 
out in each such tree forces use of another system to do so, just 
imposing different difficulties for this use case. I note that 1) code 
must be ADDED to git to prevent such duplicate checkouts which otherwise 
cause no difficulty to git itself, and 2) adding those checks requires 
additional work to avoid the fallout. I have yet to hear what the upside 
of such a restriction is, I only see downsides.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH] worktree: replace checkout --to with worktree new

2015-06-30 Thread Mark Levedahl

On 06/30/2015 06:11 PM, Eric Sunshine wrote:

On Tue, Jun 30, 2015 at 12:56 AM, Eric Sunshine sunsh...@sunshineco.com wrote:

The command git checkout --to path is something of an anachronism,
encompassing functionality somewhere between checkout and clone.
The introduction of the git-worktree command, however, provides a proper
and intuitive place to house such functionality. Consequently,
re-implement git checkout --to as git worktree new.
[...]
Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---
This is primarily a code and documentation relocation patch, with minor
new code added to builtin/worktree.c. Specifically:

* builtin/worktree.c:new() is new. It recognizes a --force option (git
   worktree new --force path branch) which allows a branch to be
   checked out in a new worktree even if already checked out in some
   other worktree (thus, mirroring the functionality of git checkout
   --ignore-other-worktrees).


Speaking of git worktree new --force, should we revisit git
checkout --ignore-other-worktrees before it gets set in stone? In
particular, I'm wondering if it makes sense to overload git-checkout's
existing --force option to encompass the functionality of
--ignore-other-worktrees as well. I don't think there would be any
semantic conflict by overloading --force, and I do think that --force
is more discoverable and more intuitive.



I agree with -f subsuming --ignore...:  -f/--force should really mean 
do this if at all possible, not just ignore some checks. Similar to 
rm -f, etc.


Maintaining --ignore-other-worktrees, and making that a configurable 
option (worktree.ignoreothers??) would allow selectively ignoring just 
this one issue, perhaps permanently, but not the others -f already 
overrides. This would make sense if other options were added to ignore 
other subsets of checks that can block a checkout, probably not otherwise.



Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] nd/multiple-work-trees updates

2015-02-12 Thread Mark Levedahl

On 02/12/2015 06:51 PM, Jens Lehmann wrote:

Am 13.02.2015 um 11:57 schrieb Junio C Hamano:

Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:


These patches are on top of what's in 'pu'. They add
--ignore-other-worktrees and make a note about current submodule
support status. I don't think submodule support is ready yet even
with Max Kirillov's series [1]. His 03/03 is already fixed in 'pu'
though, so only 01/03 and 02/03 are new.

[1] http://thread.gmane.org/gmane.comp.version-control.git/261107


With the understanding (perhaps a strongly-worded paragraph in the
release notes) that this is not suitable for submodule users yet,
is this in a good enough shape to go to 'next'?


No objections from my side (and maybe we should also add a warning
that *all* worktree-related configuration - e.g. EOL options - are
currently always shared between all worktrees).

Adding submodule support can then be done in another series (and
renaming core.worktree to something else is definitely *not* the
way to do that! ;-).

I concur the patch series is good enough for next. Better multiple 
worktree support for submodules is, I think, a sizeable topic that will 
take a while to settle, so should be worked after this base is integrated.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] git-checkout.txt: a note about multiple checkout support for submodules

2015-01-04 Thread Mark Levedahl

On 01/03/2015 04:41 AM, Nguyễn Thái Ngọc Duy wrote:

The goal seems to be using multiple checkouts to reduce disk space.
But we have not reached an agreement how things should be. There are a
couple options.

  - You may want to keep $SUB repos elsewhere (perhaps in a central
place) outside $SUPER. This is also true for nested submodules
where a superproject may be a submodule of another superproject.


This is my preference: I keep a tree of bare git repos outside of all 
work areas, and use new-workdir to create trees of workdirs as needed. I 
explored trying to keep $SUB repos in others (including mods to 
submodule / new-workdir to manage this), found this really leads to too 
much complication compared to just having a set of bare repos elsewhere. 
This bare repo approach also has the advantage that no particular 
workdir is special, all workdirs that point to the same gitdir are equal.


Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] checkout: add --ignore-other-wortrees

2015-01-04 Thread Mark Levedahl

On 01/03/2015 04:41 AM, Nguyễn Thái Ngọc Duy wrote:

Noticed-by: Mark Levedahl mleved...@gmail.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
  Documentation/git-checkout.txt | 6 ++
  builtin/checkout.c | 6 +-
  t/t2025-checkout-to.sh | 7 +++
  3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 0c13825..52eaa48 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -232,6 +232,12 @@ section of linkgit:git-add[1] to learn how to operate the 
`--patch` mode.
specific files such as HEAD, index... See MULTIPLE WORKING
TREES section for more information.
  
+--ignore-other-worktrees::

+   `git checkout` refuses when the wanted ref is already checked
+   out by another worktree. This option makes it check the ref
+   out anyway. In other words, the ref can be held by more than one
+   worktree.
+

Thanks for adding this, I haven't had a chance to test but by reading 
this solves the problem I raised.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 24/34] checkout: reject if the branch is already checked out elsewhere

2014-12-03 Thread Mark Levedahl

On 12/02/2014 12:30 PM, Junio C Hamano wrote:

Duy Nguyen pclo...@gmail.com writes:


FWIW git-branch usually can show the original branch of detached head
(must not always). I don't think we have a plumbing equivalent for it
though. People can tail -1 $GIT_DIR/logs/HEAD| sed .. but that seems
hacky.

@{-1}, i.e. the last branch I checked out?


I do like read-only ref concept where we can keep ref name
(especially tags) in HEAD until the next commit. But it didn't go
anywhere

Remind me.  That sounds somewhat interesting.


I think these ideas support solutions more complicated than the 
problem trying to be solved. Consider a use case: multiple algorithms, 
each a different branch in one repository, any of which can be used to 
analyze the same kinds of data. We also have multiple data sets, each a 
separate branch in a other repository. For development / test / analysis 
it is necessary to have multiple checked out pairs (algorithm + data) on 
the same machine to allow comparison / debugging in place of different 
combinations. Assume one algorithm, multiple data sets are checked out 
and being worked on.


With new-workdir, or Duy's approach with --ignore-other-checkouts, all 
are checked out normally in git, git-branch, git-status, git log all 
work normally. If a change needs to be made that affects the branch in 
more than one checked out repository, once done the other copies are out 
of date. It does not matter which instance is modified, once committed 
the change is already visible in all others, and git reset --hard in 
all the others completes the process. This is not difficult to 
understand, requires no new code, no special methods.


Consider the alternatives:
a) Use separate complete repositories + work trees: now the new branch 
needs to be broadcast to all using fetch or pull, and as the change 
might have been an amend, fetch + reset --hard may be required. This is 
not simpler to implement in practice, nor do I find it easier to 
explain. Note also that if using push, it is possible to force push into 
the current branch of the other copies, with receive.denyCurrentBranch = 
false, resulting in exactly the same situation as above using 
new-workdir (checked out code not matching the ref).


b) Use Duy's approach without --ignore-other-checkouts. First, you have 
to find the copy that is not on a detached HEAD, detach the HEAD their, 
then go back to the copy where the problem is found, attach the HEAD in 
that one, and make the change. Then go back and do git reset --hard 
$branch in the others.


I just don't see how these alternatives are in the end any simpler to 
use or explain.



Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 24/34] checkout: reject if the branch is already checked out elsewhere

2014-11-30 Thread Mark Levedahl

On 11/30/2014 03:24 AM, Nguyễn Thái Ngọc Duy wrote:

One branch obviously can't be checked out at two places (but detached
heads are ok). Give the user a choice in this case: --detach, -b
new-branch, switch branch in the other checkout first or simply 'cd'
and continue to work there.



This seems too restrictive and is not obvious to me: I currently use 
git-new-workdir to have multiple checkouts of the same branch, with no 
ill effect. While those who do not understand what is going on 
underneath might be confused by one checkout suddenly showing 
uncommitted diffs, I don't accept that as a reason to outright prevent 
such use. I suggest, at the very least, that this behavior be overridden 
by a --force flag?


Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: inform pre-commit if --amend is used

2014-11-28 Thread Mark Levedahl

On 11/28/2014 12:18 AM, Jeff King wrote:

On Thu, Nov 27, 2014 at 09:40:08AM -0500, Mark Levedahl wrote:


Then when you add new arguments, the hook has to search through the
parameters looking for one that matches, rather than just checking $1
for amend (and $2 for the new option, and so on). As long as the set
of options remains relatively small, I think that is preferable.

We could also just pass them through the environment, which gives nice
named parameters.


See http://comments.gmane.org/gmane.comp.version-control.git/148479 for an
earlier conversation on this exact topic. Also, see
http://permalink.gmane.org/gmane.comp.version-control.git/148480 for a
similar change in git-gui.

Thanks for the links; I had no recollection of that thread.
Unsurprisingly, I like the HEAD/HEAD~1 suggestion. That peff guy
seems really clever (and handsome, too, I'll bet).

I'd still be OK with any of the suggestions given in this thread,
though.

-Peff
ars


Apparently our combined handsome-foo was insufficient to get this 
accepted way back when, hopefully the current submitter has more :^)


In any event, I've carried the patches using HEAD/HEAD~1 in my tree for 
the last 4+ years, have a widely used pre-commit script that depends 
upon those. So, I personally would be very happy to see this finally 
show up in Junio's tree, would prefer HEAD/HEAD~1 but can adapt to whatever.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] commit: inform pre-commit if --amend is used

2014-11-27 Thread Mark Levedahl

On 11/25/2014 12:03 AM, Jeff King wrote:

On Mon, Nov 24, 2014 at 08:58:37PM -0800, Junio C Hamano wrote:


Jeff King p...@peff.net writes:


   1. It is a bit more obvious when debugging or dumping arguments (e.g.,
  via GIT_TRACE), especially if new options are added after the
  first.

   2. It makes it easier for a script to work on old and new versions of
  git. It sees either amend or noamend for the two obvious cases,
  and if it sees no argument, then it knows that it does not know
  either way (it is running on an old version of git).

  Technically one can tell the difference in shell between an empty
  string and a missing argument, but it is sufficiently subtle that I
  think noamend is a better route.


If we ever add more info, would we want to keep piling on new
arguments, though?  Wouldn't it a viable option to use amend vs
not giving anything (not even an empty string), so that normal case
there won't be no parameter?


Then when you add new arguments, the hook has to search through the
parameters looking for one that matches, rather than just checking $1
for amend (and $2 for the new option, and so on). As long as the set
of options remains relatively small, I think that is preferable.

We could also just pass them through the environment, which gives nice
named parameters.

-Peff



See http://comments.gmane.org/gmane.comp.version-control.git/148479 for 
an earlier conversation on this exact topic. Also, see 
http://permalink.gmane.org/gmane.comp.version-control.git/148480 for a 
similar change in git-gui.


-Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] remote.c - Make remote definition require a url

2014-10-13 Thread Mark Levedahl

On 10/13/2014 01:19 PM, Junio C Hamano wrote:

Mark Levedahl mleved...@gmail.com writes:


Some options may be configured globally for a remote (e.g, tagopt).

Or some remotes may have only pushurl and not url.  git remote
output for me has a few such remotes but wouldn't this patch break
it?

If a caller that walks the list of remotes misbehaves only because
it assumes that r-url always is always valid, isn't that assumption
what needs to be fixed?  for_each_remote() should be kept as a way
to enumerate all the [remote foo], I would think.




As long as the rule is that for_each_remote will enumerate every remote 
that has anything defined at all, even if only in the global config 
outside of a user's control, I'm not really sure how to tell whether the 
missing url / pushurl / whatever is intentional, or a misconfiguration, 
so having the code complain that it didn't find what it wanted (the 
current condition) is probably no worse than the alternatives. Patch 
withdrawn.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] remote.c - Make remote definition require a url

2014-10-11 Thread Mark Levedahl
Some options may be configured globally for a remote (e.g, tagopt).
The presence of such options in a global config should not cause
git remote or get fetch to believe that remote is configured
for every repository. Change to require definition of remote.foo.url
for the remote to be included in git fetch --all or git remote
update.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 remote.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index ce785f8..1b08924 100644
--- a/remote.c
+++ b/remote.c
@@ -761,7 +761,7 @@ int for_each_remote(each_remote_fn fn, void *priv)
read_config();
for (i = 0; i  remotes_nr  !result; i++) {
struct remote *r = remotes[i];
-   if (!r)
+   if (!r || !r-url)
continue;
if (!r-fetch)
r-fetch = parse_fetch_refspec(r-fetch_refspec_nr,
-- 
2.1.2.2.0.14

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-completion.bash - avoid excruciatingly slow ref completion on Cygwin

2014-10-11 Thread Mark Levedahl
$git checkout tab was taking about 3.5 seconds to respond on one
repository having four remotes with about 100 total refs (measured on
Cygwin).  All of the time was being claimed in git for-each-ref to do
its work.  This working directory was created using git-new-workdir, and
thus .git/refs and .git/packed-refs are both symlinks.  for-each-ref
operates in a way that causes the .git/refs symlink to be resolved
multiple times for each ref in the repository, and Cygwin is especially
slow in such operations.

Patching refs.c to avoid repeatedly dereferencing the symlink reduced
execution time from about 3.5 seconds to about 1.1 seconds (but no
improvement on Linux), while an alternate approach of replacing the
ref-list expansion with a shell pipeline provides a larger improvement on
Cygwin and also improves Linux.  So, the shell pipeline approach is
provided here.

Relevant timing results using the same repository on both Linux and
Cygwin:

On Cygwin:

$ time git for-each-ref --format=%(refname:short) refs

real0m3.523s
user0m0.436s
sys 0m2.733s

$ time (cd $GIT_DIR ; cat packed-refs ; find refs/ -type f) \
2/dev/null | sed -ne 's@^.*refs/@refs/@p' | sort | uniq

real0m0.503s
user0m0.307s
sys 0m0.139s

On Linux (essentially the same hardware):

$ time git for-each-ref --format=%(refname:short) refs

real0m0.020s
user0m0.006s
sys 0m0.014s

$ time (cd $GIT_DIR ; cat packed-refs ; find refs/ -type f) \
2/dev/null | sed -ne 's@^.*refs/@refs/@p' | sort | uniq

real0m0.012s
user0m0.006s
sys 0m0.005s

So, this is a win even on Linux, but more importantly it makes use of
tab completion tolerable on Cygwin when symlinks are involved.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 contrib/completion/git-completion.bash | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 965778e..62d976e 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -319,8 +319,9 @@ __git_heads ()
 {
local dir=$(__gitdir)
if [ -d $dir ]; then
-   git --git-dir=$dir for-each-ref --format='%(refname:short)' \
-   refs/heads
+   (cd $dir ; cat packed-refs ; find refs/heads -type f) 
2/dev/null |
+   sed -ne 's@^.*refs/heads/@@p' |
+   sort -u
return
fi
 }
@@ -329,8 +330,9 @@ __git_tags ()
 {
local dir=$(__gitdir)
if [ -d $dir ]; then
-   git --git-dir=$dir for-each-ref --format='%(refname:short)' \
-   refs/tags
+   (cd $dir ; cat packed-refs ; find refs/tags -type f) 
2/dev/null |
+   sed -ne 's@^.*refs/tags/@@p' |
+   sort -u
return
fi
 }
@@ -348,17 +350,21 @@ __git_refs ()
format=refname
refs=${cur%/*}
track=
+   (cd $dir ; cat packed-refs ; find refs/ -type f) 
2/dev/null |
+   sed -ne 's@^.*refs/@refs/@p' |
+   sort -u
+   return
;;
*)
for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
if [ -e $dir/$i ]; then echo $i; fi
done
-   format=refname:short
-   refs=refs/tags refs/heads refs/remotes
+   (cd $dir ; cat packed-refs ; find refs/ -type f) 
2/dev/null |
+   sed -rne 's@^.*refs/(heads|remotes|tags)/@@p' |
+   sort -u
+   return
;;
esac
-   git --git-dir=$dir for-each-ref --format=%($format) \
-   $refs
if [ -n $track ]; then
# employ the heuristic used by git checkout
# Try to find a remote branch that matches the 
completion word
-- 
2.1.2.2.0.14

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Jul 2013, #09; Mon, 29)

2013-08-03 Thread Mark Levedahl

On 08/01/2013 05:12 PM, Junio C Hamano wrote:

Ramsay Jones ram...@ramsay1.demon.co.uk writes:


Junio C Hamano wrote:

Ramsay Jones ram...@ramsay1.demon.co.uk writes:


  I am personally in favor of this simpler solution.  Comments?

I had expected this to me marked for 'master'.

Has this simply been overlooked, or do you have reservations about
applying this patch?

I am just being careful and do want to keep it cooking in 'next'
during the feature freeze.  The more users work with 'next' (not
work *on* 'next'), the more confidence we would be with, and
hopefully this can be one of the topis that graduate early after
the 1.8.4 release.

Hmm, this patch is a bug-fix for a bug that (currently) will be
_introduced_ by v1.8.4.

OK, let's merge it then.  Thanks for being patient with me.


Do you want me to try and find a different bug-fix for v1.8.4?
(Although that would most likely be more risky than simply taking
this patch! ;-) ).

Absolutely not, and I 100% agree with you.

I have been using this patch since Ramsey first sent it, have noticed no 
trouble over that time but all of my work is with filemode=true (has 
been since I started using git as Cygwin is a secondary platform for me 
and interoperability with repos on Linux is an absolute requirement).


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Cygwin has trustable filemode

2013-07-22 Thread Mark Levedahl

On 07/22/2013 01:02 AM, Junio C Hamano wrote:

b) The Cygwin project has always shipped git binaries built without
NO_TRUSTABLE_FILEMODE

That is a fair point.  So let's do this instead.

-- 8 --
From: Mark Levedahl mleved...@gmail.com
Subject: [PATCH] cygwin: stop forcing core.filemode=false

We force core.filemode=false since c869753e (Force core.filemode to
false on Cygwin., 2006-12-30), even when the repository is on a
filesystem on which Cygwin can give us trustable filemodes, because
many native Windows applications the users use to edit files in the
working tree tend to (re)create files with executable bit randomly
set or reset.  However, binary distribution of Git that is supplied
by the downstream project to its users has been built without this
consideration.

Drop NO_TRUSTABLE_FILEMODE from our default configuration so that
hand-compiled Git out of box will match theirs.

Signed-off-by: Mark Levedahl mleved...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
  config.mak.uname | 1 -
  1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..779d06a 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-   NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.

ok by me.

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Cygwin has trustable filemode

2013-07-21 Thread Mark Levedahl

On 07/21/2013 05:56 PM, Junio C Hamano wrote:

Ramsay Jones ram...@ramsay1.demon.co.uk writes:


Mark Levedahl wrote:

The supported Cygwin distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default.
...
Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus
dropped support for all OS variants that lack NTFS and/or ...
...  Thus, POSIX filemode support
could not be expected by default on a Cygwin 1.5 installation, but is
expected by default on a 1.7 installation.


Again, I have to ask; should you not revert commit c869753e (Force 
core.filemode
to false on Cygwin., 30-12-2006)?  After this commit, there is no longer any 
user
of the NO_TRUSTABLE_FILEMODE build variable, and no real prospect of anyone else
wanting to use it.

Thanks for raising this point.

Reading c869753e once again:

 The issue is that Cygwin and NTFS correctly supports the
 executable mode bit, and Git properly detected that, but most
 native Windows applications tend to create files such that
 Cygwin sees the executable bit set when it probably shouldn't
 be.

In other words, the reason why NO_TRUSTABLE_FILEMODE was added was
not because the Cygwin did not give us reliable filemodes.  It was
because tools outside the control of Git and/or Cygwin that users
use tend to misbehave, even when the working tree is on a filesystem
on which Cygwin can give us trustable filemodes.

So 1.7 always supports core.filemodes correctly because it no
longer works on filesystems without trustable filemodes is not a
valid reason to justify Mark's change.

There are only three possible ways going forward, I think:

  (A) Drop Mark's patch, and do nothing else, because breakages of
  other people's programs are not fixed by Cygwin 1.7's improved
  filesystem support, and users still use those mode breaking
  programs written by others;

  (B) Drop Mark's patch, and revert c869753e, because it is not the
  business of our project to sweep breakages of other people's
  tools under the rug by crippling our software; or

  (C) Drop NO_TRUSTABLE_FILEMODE for _all_ versions of Cygwin,
  declaring that the spirit of c869753e to work around bugs in
  other people's software by crippling Git is justified, but that
  it is no longer necessary on Cygwin because people do not use
  such misbehaving third-party tools anymore.

These three each rely on its own precondition; I suspect it is
likely that (A)'s is the most accurate description of the real world.



Perhaps the simplest approach is to just defer to the judgement of the 
Cygwin project maintainers here.


a) The Cygwin project has its stated objective of being as matching Linux.
b) The Cygwin project has always shipped git binaries built without 
NO_TRUSTABLE_FILEMODE


Also - users who do not want Cygwin's assumptions / environment are now 
free to use the msysgit version and frankly they should be so encouraged 
- it is faster than Cygwin's git. This option was not available in 2006.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Cygwin has trustable filemode

2013-07-19 Thread Mark Levedahl
The supported Cygwin distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus 
dropped support for all OS variants that lack NTFS and/or the full win32 
api, and since late 1.5 development, Cygwin maps POSIX modes to NTFS ACLs 
by default.  Cygwin 1.5 supported OS variants that used FAT as the native 
file system, and had optional methods for providing POSIX file modes on 
top of FAT12/16 and NTFS, though not FAT32.  Also, support for POSIX modes 
on top of FAT were dropped later in 1.5.  Thus, POSIX filemode support 
could not be expected by default on a Cygwin 1.5 installation, but is 
expected by default on a 1.7 installation.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
Junio - The above notes are more accurate than in my previous commit message,
so if this commit survives into next/master, I would prefer this version as
opposed to the one now on pu (da875762)

Mark

 config.mak.uname | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 174703b..bf5db47 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -164,7 +164,6 @@ ifeq ($(uname_O),Cygwin)
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-   NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-19 Thread Mark Levedahl

On 07/18/2013 07:32 PM, Junio C Hamano wrote:

Mark Levedahl mleved...@gmail.com writes:


Unlike the results on the fast Win7 laptop, the above show
statistically significant slow down from the fast_lstat approach. I'm
just not seeing a case for the special case handling, and of course
Junio has already voted with his preference of removing the special
case stuff as well.

Please don't take what I said as any vote in this thread.  I do
not have a first-hand data to back anything up.

I was primarily trying to see my understanding of the consensus of
the thread was correct. If we can do without s/lstat/fast_lstat/
almost everywhere in the codebase, of course, I would be happier, as
it would give us one less thing to worry about.

If the assumptions like they were declining minority and only lose
population over time, it is easy for them to revert the removal
and keep going, and removal will not hurt them too much in the
first place, only a few hundred milliseconds, that might trump the
longer-term maintainability issue, and we may end up having to carry
that win32 stat implementation a bit longer until these users all
switch to Cygwin 1.7, but judging from the cvs binary seems to be
built incorrectly incident the other day, it might be the case some
users still hesitate to update, fearing that 1.7 series may not be
solid enough, perhaps?



I cannot say how many users of 1.5 exist. I see no evidence of 1.5 users 
on the Cygwin lists, the developers noted a total of 14 downloads of the 
1.5 installer in the year prior to removal of 1.5 from the mirrors. The 
stated reason for keeping 1.5 available for four years after its 
development stopped was support of older Windows variants (which 
Microsoft dropped support of before Cygwin did, BTW). But, none of this 
is conclusive about the current relevance of v 1.5.


The status as I understand things:
1) The existing schizophrenic stat on master is incompatible with the 
new reference api on pu, therefore some change is required.
2) Ramsay has graciously provided two separate patches to address the 
above, one reverting to use only of cygwin stat/lstat, one including a 
fast_lstat that should provide better speed at the expense of POSIX 
compliance.
3) We have conflicting reports about the speed of the second patch: 
Ramsay shows a good speed up on Cygwin 1.5, with slight performance 
regrets on MINGW, no change on Linux. I found no effect on a current 
bare-metal Window 7 installation using Cygwin 1.7, but degradation on a 
virtualized WinXP installation using Cygwin 1.7. Ramsay also showed a 
significant performance difference between running from the git tree vs 
being installed, I looked for this effect but failed to replicate it.


The maintenance argument between the two patches is clear, the 
performance argument less so. Perhaps others can help clarify this.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Cygwin has trustable filemode

2013-07-19 Thread Mark Levedahl

On 07/19/2013 12:40 PM, Junio C Hamano wrote:

Thanks, will replace.

What do we want to do with the compat/regex build-time switch?

IIRC, this was only needed for 1.7 and not 1.5, and I also would
expect (without anything to back-up, so this is more a faith than
expectation) over time the new library would have a working regex
library.



The situation is that Cygwin uses newlib rather than glibc, and does so 
for licesnsing reasons (redhat sells licenses to developers allowing 
closed source applications built using Cygwin). So, there must be a 
compelling need to fix the library - git has a simple work around, so 
isn't the case. Also, Cygwin has a perl regex library for those 
demanding more complete / correct regex solution. So, I make no 
prediction on when the newlib regex functions are fixed.


Related: Should we have separate settings for 1.5 and 1.7 for several 
variables? Conflicts I see not reflected in config.mak.uname on pu:

trustable filemode   (1.7 has, 1.5 does not)
MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP 
utility is convolved in this)

regex - 1.7 is broken, per Ramsay 1.5 works

If you think its worth it, I'll create a patch series with the above and 
justifications for the different settings that I know.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] Cygwin 1.7 has thread-safe pread

2013-07-19 Thread Mark Levedahl
Per http://cygwin.com/ml/cygwin/2012-07/msg00331.html , cygwin 1.7
was modified to explicitly support git's use of pread, so make this
the default. Do not affect earlier cygwin versions.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 8652da9..048c252 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -160,10 +160,10 @@ ifeq ($(uname_O),Cygwin)
NO_IPV6 = YesPlease
NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
OLD_ICONV = UnfortunatelyYes
+   NO_THREAD_SAFE_PREAD = YesPlease
else
NO_REGEX = UnfortunatelyYes
endif
-   NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/4] Cygwin 1.7 needs compat/regex

2013-07-19 Thread Mark Levedahl
Cygwin v1.7 uses the regex library from newlib which does not pass git's
tests, so don't use it. This fixes failures in t4018 and t4034.

Continue to use the platform supplied regex library for earlier versions.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 config.mak.uname | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/config.mak.uname b/config.mak.uname
index 104dc44..8652da9 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -160,6 +160,8 @@ ifeq ($(uname_O),Cygwin)
NO_IPV6 = YesPlease
NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
OLD_ICONV = UnfortunatelyYes
+   else
+   NO_REGEX = UnfortunatelyYes
endif
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/4] Cygwin 1.7 has trustable filemode

2013-07-19 Thread Mark Levedahl
The current Cygwin 1.7 distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way. Cygwin 1.5
installations are less likely to have this support, so leave the old
default in place for those.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Historical notes: Cygwin version 1.7 supports Windows-XP and newer, thus
dropped support for all OS variants that lack NTFS and/or the full win32
api, and since late 1.5 development, Cygwin maps POSIX modes to NTFS ACLs
by default.  Cygwin 1.5 supports OS variants that use FAT as the native
file system, and had optional methods for providing POSIX file modes on
top of FAT12/16 and NTFS, though not FAT32.  Also, support for POSIX modes
on top of FAT were dropped later in 1.5.  Thus, POSIX filemode support
is not expected by default on a Cygwin 1.5 installation, but is expected
by default on a 1.7 installation.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 config.mak.uname | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..104dc44 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -158,12 +158,12 @@ ifeq ($(uname_O),Cygwin)
NO_MKSTEMPS = YesPlease
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
+   NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
OLD_ICONV = UnfortunatelyYes
endif
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-   NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/4] Cygwin 1.7 supports mmap

2013-07-19 Thread Mark Levedahl
git has shipped for years with MMAP enabled in the stock distribution,
there are no reports of problems / failures on the list relating to
this. Leave the default as-is on v1.5 due to lack of knowlege of this
working on earlier Cygwin.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 config.mak.uname | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/config.mak.uname b/config.mak.uname
index 048c252..32e8332 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -161,16 +161,16 @@ ifeq ($(uname_O),Cygwin)
NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
OLD_ICONV = UnfortunatelyYes
NO_THREAD_SAFE_PREAD = YesPlease
+   # There are conflicting reports about this.
+   # On some boxes NO_MMAP is needed, and not so elsewhere.
+   # Try commenting this out if you suspect MMAP is more efficient
+   NO_MMAP = YesPlease
else
NO_REGEX = UnfortunatelyYes
endif
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
-   # There are conflicting reports about this.
-   # On some boxes NO_MMAP is needed, and not so elsewhere.
-   # Try commenting this out if you suspect MMAP is more efficient
-   NO_MMAP = YesPlease
X = .exe
COMPAT_OBJS += compat/cygwin.o
UNRELIABLE_FSTAT = UnfortunatelyYes
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Cygwin has trustable filemode

2013-07-19 Thread Mark Levedahl

On 07/19/2013 03:16 PM, Junio C Hamano wrote:

Mark Levedahl mleved...@gmail.com writes:


Related: Should we have separate settings for 1.5 and 1.7 for several
variables? Conflicts I see not reflected in config.mak.uname on pu:
 trustable filemode   (1.7 has, 1.5 does not)
 MMAP/Pread (1.7 pread is thread safe, 1.5 I dont think was, MMAP
utility is convolved in this)
 regex - 1.7 is broken, per Ramsay 1.5 works

If you think its worth it, I'll create a patch series with the above
and justifications for the different settings that I know.

I'd say that would be a sensible thing to do, given that the
alternative seems to be let's drop 1.5 support right now, because
otherwise we cannot run Git on 1.7.


Ok, the following sequence builds up options for Cygwin 1.7 while 
leaving Cygwin 1.5 as-is. This series should replace


dad577f Cygwin has trustable filemode
174bb98 Use compat/regex on Cygwin

After merging the following into current pu, all tests that run by 
default pass on Cygwin 1.7, i.e.

prove -j 8 t[0-9]*.sh
reports All tests successful.
I've *never* had this happen on Cygwin before.

Mark


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t3032 - make compatible with systems using \r\n as a line ending

2013-07-18 Thread Mark Levedahl

On 07/18/2013 03:19 PM, Ramsay Jones wrote:

Jonathan Nieder wrote:

Mark Levedahl wrote:


Subtests 6, 7, and 9 rely test that merge-recursive correctly
ignores whitespace when so directed. Change the particular whitespace
sequences to be ones that are not known line endings so the whitespace
is not changed when being extracted by line oriented grep.

merge-recursive needs to be able to deal with \r at EOL, too, so if at
all possible I would prefer to see the test fixed to pass on Cygwin
some other way.

Maybe use -U/--binary option to grep? Indeed, if you look at the top of
that test file, you will see:

 test_have_prereq SED_STRIPS_CR  SED_OPTIONS=-b
 test_have_prereq MINGW  export GREP_OPTIONS=-U

which may explain why it works for me on MinGW, but not why it works on
cygwin 1.5.

ATB,
Ramsay Jones






Thanks, hadn't noticed that, it leads to a much better patch.

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] test-lib.sh - define and use GREP_STRIPS_CR

2013-07-18 Thread Mark Levedahl
Define a common macro for grep needing -U to allow tests to not need
to inquire of specific platforms needing this option. Change
t3032 and t5560 to use this rather than testing explicitly for mingw.
This fixes these two tests on Cygwin.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
This replaces my earlier patch against t3032 (8896b287 on pu)

 t/t3032-merge-recursive-options.sh | 2 +-
 t/t5560-http-backend-noserver.sh   | 2 +-
 t/test-lib.sh  | 2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t3032-merge-recursive-options.sh 
b/t/t3032-merge-recursive-options.sh
index 2b17311..5fd7bbb 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-options.sh
@@ -14,7 +14,7 @@ test_description='merge-recursive options
 . ./test-lib.sh
 
 test_have_prereq SED_STRIPS_CR  SED_OPTIONS=-b
-test_have_prereq MINGW  export GREP_OPTIONS=-U
+test_have_prereq GREP_STRIPS_CR  export GREP_OPTIONS=-U
 
 test_expect_success 'setup' '
conflict_hunks () {
diff --git a/t/t5560-http-backend-noserver.sh b/t/t5560-http-backend-noserver.sh
index ef98d95..9be9ae3 100755
--- a/t/t5560-http-backend-noserver.sh
+++ b/t/t5560-http-backend-noserver.sh
@@ -5,7 +5,7 @@ test_description='test git-http-backend-noserver'
 
 HTTPD_DOCUMENT_ROOT_PATH=$TRASH_DIRECTORY
 
-test_have_prereq MINGW  export GREP_OPTIONS=-U
+test_have_prereq GREP_STRIPS_CR  export GREP_OPTIONS=-U
 
 run_backend() {
echo $2 |
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2d63307..1abea40 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -824,6 +824,7 @@ case $(uname -s) in
test_set_prereq MINGW
test_set_prereq NOT_CYGWIN
test_set_prereq SED_STRIPS_CR
+   test_set_prereq GREP_STRIPS_CR
;;
 *CYGWIN*)
test_set_prereq POSIXPERM
@@ -831,6 +832,7 @@ case $(uname -s) in
test_set_prereq NOT_MINGW
test_set_prereq CYGWIN
test_set_prereq SED_STRIPS_CR
+   test_set_prereq GREP_STRIPS_CR
;;
 *)
test_set_prereq POSIXPERM
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-18 Thread Mark Levedahl

On 07/18/2013 05:49 PM, Torsten Bögershausen wrote:

On 2013-07-18 19.50, Ramsay Jones wrote:


Hmm, that looks good. :-D

Torsten reported a performance boost using the win32 stat() implementation
on a linux git repo (2s - 1s, if I recall correctly) on cygwin 1.7.
Do you have a larger repo available to test?

(I have a 5 years old Dual Core, 2.5 Ghz, 1 TB hard disk, Win XP, cygwin 1.7)
On that machine I can see the performance boost.
Which kind of computers are you guys using?

SSD/hard disk ?
How much RAM ?
Which OS ?
Is there a difference between Win XP, Win7, Win8?

[snip]


My previous results were from a Win 7 laptop, 2.7 GHz 2nd generation I7, 
8 Gig Ram, 250 GByte spinning rust drive, all formatted NTFS.


Here's some more results, running WinXP in VirtualBox on my older Linux 
laptop (2.5 GHz Penryn dual core, 500 GByte spinning rust, virtual file 
system is NTFS). First, results using Ramsay's last patch on pu adding 
the fast_lstat: Timing results are after first doing 5 'git status runs' 
to assure the cache is hot:


% using the fast_lstat and friends...
/usr/local/src/gittime git -c core.filemode=false status  /dev/null

real0m0.469s
user0m0.062s
sys 0m0.436s
/usr/local/src/git

/usr/local/src/gittime git -c core.filemode=true status  /dev/null

real0m0.719s
user0m0.030s
sys 0m0.686s
/usr/local/src/git

And now the same. but using Ramsay's first patch that removes all win32 
stat stuff and forces everything to go through Cygwin's normal stat/fstat:

% stat - with / without core.filemode, no win32 stats
/usr/local/src/gittime git -c core.filemode=false status  /dev/null

real0m0.328s
user0m0.093s
sys 0m0.264s
/usr/local/src/git

/usr/local/src/gittime git -c core.filemode=true status  /dev/null

real0m0.625s
user0m0.124s
sys 0m0.500s
/usr/local/src/git


Unlike the results on the fast Win7 laptop, the above show statistically 
significant slow down from the fast_lstat approach. I'm just not seeing 
a case for the special case handling, and of course Junio has already 
voted with his preference of removing the special case stuff as well.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t6131 - skip tests if on case-insensitive file system

2013-07-17 Thread Mark Levedahl
This test fails on Cygwin where the default system configuration does not 
support case sensitivity (only case retention), so don't run the test on 
such systems.  

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 t/t6131-pathspec-icase.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh
index 3215eef..8d4a7fc 100755
--- a/t/t6131-pathspec-icase.sh
+++ b/t/t6131-pathspec-icase.sh
@@ -3,6 +3,12 @@
 test_description='test case insensitive pathspec limiting'
 . ./test-lib.sh
 
+if test_have_prereq CASE_INSENSITIVE_FS
+then
+   skip_all='skipping case sensitive tests - case insensitive file system'
+   test_done
+fi
+
 test_expect_success 'create commits with glob characters' '
test_commit bar bar 
test_commit bAr bAr 
-- 
1.8.3.2.0.63

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


t3032 incompatible with Cygwin/Windows

2013-07-16 Thread Mark Levedahl
Subtests 6,7, and 9 of t3032 fail on Cygwin, and I presume will fail on 
msysgit for similar reasons. Looking at test 6, the expected result is a 
line ending with \r\n in text.txt. This line is extracted with grep 
(grep 'justice and holiness' text.txt  actual), with unavoidable result 
that on Cygwin the line ending is \n. This happens because on Cygwin, 
the text utils are compiled to open files in text mode meaning than \n 
and \r\n are both recognized as EOL markers. Thus, even though text.txt 
is an exact match for what is created on Linux, the test fails because 
\r\n cannot be distinguished by the available tools.


I'm not sure the right way forward. I did confirm that by substituting 
q_to_tab for q_to_cr in t3032, the test pass on Cygwin and on Linux. 
Perhaps t3032 should be so amended to avoid use of a non-portable line 
ending construct?


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t3032 - make compatible with systems using \r\n as a line ending

2013-07-16 Thread Mark Levedahl
Subtests 6, 7, and 9 rely test that merge-recursive correctly
ignores whitespace when so directed. Change the particular whitespace
sequences to be ones that are not known line endings so the whitespace
is not changed when being extracted by line oriented grep.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 t/t3032-merge-recursive-options.sh | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t3032-merge-recursive-options.sh 
b/t/t3032-merge-recursive-options.sh
index 2b17311..52e275c 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-options.sh
@@ -92,7 +92,7 @@ test_expect_success 'setup' '
s/Polemarchus interposing./Polemarchus, interposing.Q/
/justice and holiness/ s/$/Q/
/pay your debts/ s/$/Q/
-text.txt | q_to_cr text.txt+ 
+text.txt | q_to_tab text.txt+ 
mv text.txt+ text.txt 
git commit -a -m Clarify 
git show-branch --all
@@ -125,7 +125,7 @@ test_expect_success '-Xignore-space-change makes 
cherry-pick succeed' '
 '
 
 test_expect_success '--ignore-space-change: our w/s-only change wins' '
-   q_to_cr -\EOF expected 
+   q_to_tab -\EOF expected 
justice and holiness and is the nurse of his age and theQ
EOF
 
@@ -150,7 +150,7 @@ test_expect_success '--ignore-space-change: does not ignore 
new spaces' '
cat -\EOF expected1 
Well said, Cephalus, I replied; but as con cerning justice, what is
EOF
-   q_to_cr -\EOF expected2 
+   q_to_tab -\EOF expected2 
un intentionally; and when he departs to the world below he is not inQ
EOF
 
@@ -174,7 +174,7 @@ test_expect_success '--ignore-all-space drops their new 
spaces' '
 '
 
 test_expect_success '--ignore-all-space keeps our new spaces' '
-   q_to_cr -\EOF expected 
+   q_to_tab -\EOF expected 
un intentionally; and when he departs to the world below he is not inQ
EOF
 
@@ -185,7 +185,7 @@ test_expect_success '--ignore-all-space keeps our new 
spaces' '
 '
 
 test_expect_success '--ignore-space-at-eol' '
-   q_to_cr -\EOF expected 
+   q_to_tab -\EOF expected 
 HEAD
is not in his right mind; ought I to give them back to him?  No oneQ
===
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t3032 - make compatible with systems using \r\n as a line ending

2013-07-16 Thread Mark Levedahl

On 07/16/2013 02:59 PM, Jonathan Nieder wrote:

Mark Levedahl wrote:


Subtests 6, 7, and 9 rely test that merge-recursive correctly
ignores whitespace when so directed. Change the particular whitespace
sequences to be ones that are not known line endings so the whitespace
is not changed when being extracted by line oriented grep.

merge-recursive needs to be able to deal with \r at EOL, too, so if at
all possible I would prefer to see the test fixed to pass on Cygwin
some other way.

Thanks.

No line oriented tool is going to avoid this problem. I suppose someone 
with much more perl skill I possess could write a grep replacement 
explicitly using binary file modes to fix this. Or, the test could just 
check the sha1sum of text.txt against a prestored value, Or the test 
could use \r\n ONLY on systems that do not use that as a line ending 
mode. Or ...


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t3032 - make compatible with systems using \r\n as a line ending

2013-07-16 Thread Mark Levedahl
Subtests 6, 7, and 9 rely test that merge-recursive correctly
ignores whitespace when so directed. These tests create and test for
lines ending in \r\n, but as this is a valid line separator on Windows,
convert such lines in the output to avoid confusion by line-oriented
grep.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
Sorry, forgot to copy Jonathan...

 t/t3032-merge-recursive-options.sh | 22 +-
 t/test-lib-functions.sh|  4 
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/t/t3032-merge-recursive-options.sh 
b/t/t3032-merge-recursive-options.sh
index 2b17311..41ba184 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-options.sh
@@ -125,13 +125,14 @@ test_expect_success '-Xignore-space-change makes 
cherry-pick succeed' '
 '
 
 test_expect_success '--ignore-space-change: our w/s-only change wins' '
-   q_to_cr -\EOF expected 
+   cat -\EOF expected 
justice and holiness and is the nurse of his age and theQ
EOF
 
git read-tree --reset -u HEAD 
git merge-recursive --ignore-space-change HEAD^ -- HEAD remote 
-   grep justice and holiness text.txt actual 
+   cr_to_q text.txt  text.txt+ 
+   grep justice and holiness text.txt+ actual 
test_cmp expected actual
 '
 
@@ -150,14 +151,15 @@ test_expect_success '--ignore-space-change: does not 
ignore new spaces' '
cat -\EOF expected1 
Well said, Cephalus, I replied; but as con cerning justice, what is
EOF
-   q_to_cr -\EOF expected2 
+   cat -\EOF expected2 
un intentionally; and when he departs to the world below he is not inQ
EOF
 
git read-tree --reset -u HEAD 
git merge-recursive --ignore-space-change HEAD^ -- HEAD remote 
-   grep Well said text.txt actual1 
-   grep when he departs text.txt actual2 
+   cr_to_q text.txt text.txt+
+   grep Well said text.txt+ actual1 
+   grep when he departs text.txt+ actual2 
test_cmp expected1 actual1 
test_cmp expected2 actual2
 '
@@ -174,18 +176,19 @@ test_expect_success '--ignore-all-space drops their new 
spaces' '
 '
 
 test_expect_success '--ignore-all-space keeps our new spaces' '
-   q_to_cr -\EOF expected 
+   cat -\EOF expected 
un intentionally; and when he departs to the world below he is not inQ
EOF
 
git read-tree --reset -u HEAD 
git merge-recursive --ignore-all-space HEAD^ -- HEAD remote 
-   grep when he departs text.txt actual 
+   cr_to_q text.txt text.txt+ 
+   grep when he departs text.txt+ actual 
test_cmp expected actual
 '
 
 test_expect_success '--ignore-space-at-eol' '
-   q_to_cr -\EOF expected 
+   cat -\EOF expected 
 HEAD
is not in his right mind; ought I to give them back to him?  No oneQ
===
@@ -196,7 +199,8 @@ test_expect_success '--ignore-space-at-eol' '
git read-tree --reset -u HEAD 
test_must_fail git merge-recursive --ignore-space-at-eol \
 HEAD^ -- HEAD remote 
-   conflict_hunks text.txt actual 
+   cr_to_q text.txt text.txt+ 
+   conflict_hunks text.txt+ actual 
test_cmp expected actual
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..aa8e38f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -87,6 +87,10 @@ q_to_cr () {
tr Q '\015'
 }
 
+cr_to_q () {
+   tr '\015' Q
+}
+
 q_to_tab () {
tr Q '\011'
 }
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t3032 - make compatible with systems using \r\n as a line ending

2013-07-16 Thread Mark Levedahl
Subtests 6, 7, and 9 rely test that merge-recursive correctly
ignores whitespace when so directed. These tests create and test for
lines ending in \r\n, but as this is a valid line separator on Windows,
convert such lines in the output to avoid confusion by line-oriented
grep.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 t/t3032-merge-recursive-options.sh | 22 +-
 t/test-lib-functions.sh|  4 
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/t/t3032-merge-recursive-options.sh 
b/t/t3032-merge-recursive-options.sh
index 2b17311..41ba184 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-options.sh
@@ -125,13 +125,14 @@ test_expect_success '-Xignore-space-change makes 
cherry-pick succeed' '
 '
 
 test_expect_success '--ignore-space-change: our w/s-only change wins' '
-   q_to_cr -\EOF expected 
+   cat -\EOF expected 
justice and holiness and is the nurse of his age and theQ
EOF
 
git read-tree --reset -u HEAD 
git merge-recursive --ignore-space-change HEAD^ -- HEAD remote 
-   grep justice and holiness text.txt actual 
+   cr_to_q text.txt  text.txt+ 
+   grep justice and holiness text.txt+ actual 
test_cmp expected actual
 '
 
@@ -150,14 +151,15 @@ test_expect_success '--ignore-space-change: does not 
ignore new spaces' '
cat -\EOF expected1 
Well said, Cephalus, I replied; but as con cerning justice, what is
EOF
-   q_to_cr -\EOF expected2 
+   cat -\EOF expected2 
un intentionally; and when he departs to the world below he is not inQ
EOF
 
git read-tree --reset -u HEAD 
git merge-recursive --ignore-space-change HEAD^ -- HEAD remote 
-   grep Well said text.txt actual1 
-   grep when he departs text.txt actual2 
+   cr_to_q text.txt text.txt+
+   grep Well said text.txt+ actual1 
+   grep when he departs text.txt+ actual2 
test_cmp expected1 actual1 
test_cmp expected2 actual2
 '
@@ -174,18 +176,19 @@ test_expect_success '--ignore-all-space drops their new 
spaces' '
 '
 
 test_expect_success '--ignore-all-space keeps our new spaces' '
-   q_to_cr -\EOF expected 
+   cat -\EOF expected 
un intentionally; and when he departs to the world below he is not inQ
EOF
 
git read-tree --reset -u HEAD 
git merge-recursive --ignore-all-space HEAD^ -- HEAD remote 
-   grep when he departs text.txt actual 
+   cr_to_q text.txt text.txt+ 
+   grep when he departs text.txt+ actual 
test_cmp expected actual
 '
 
 test_expect_success '--ignore-space-at-eol' '
-   q_to_cr -\EOF expected 
+   cat -\EOF expected 
 HEAD
is not in his right mind; ought I to give them back to him?  No oneQ
===
@@ -196,7 +199,8 @@ test_expect_success '--ignore-space-at-eol' '
git read-tree --reset -u HEAD 
test_must_fail git merge-recursive --ignore-space-at-eol \
 HEAD^ -- HEAD remote 
-   conflict_hunks text.txt actual 
+   cr_to_q text.txt text.txt+ 
+   conflict_hunks text.txt+ actual 
test_cmp expected actual
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..aa8e38f 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -87,6 +87,10 @@ q_to_cr () {
tr Q '\015'
 }
 
+cr_to_q () {
+   tr '\015' Q
+}
+
 q_to_tab () {
tr Q '\011'
 }
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Use compat/regex on Cygwin

2013-07-16 Thread Mark Levedahl

On 07/16/2013 05:41 PM, Ramsay Jones wrote:

Mark Levedahl wrote:

Cygwin's regex library does not pass git's tests, so don't use it. This
fixes failures in t4018 and t4034.

Hmm, these tests have always passed for me on cygwin. So, this is
presumably a regression in the new-lib regex library versions used
by cygwin 1.5 and cygwin 1.7.

ATB,
Ramsay Jones



Perhaps we should just completely separate cygwin 1.5 settings from 
those for current cygwin. Extra motivation is that cygwin 1.5 is no 
longer on the mirrors - the last real update was mid 2009, and the 
project finally removed the installation tree altogether.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-16 Thread Mark Levedahl

On 07/16/2013 11:42 AM, Dmitry Potapov wrote:

On Tue, Jul 16, 2013 at 7:54 AM, Mark Levedahl mleved...@gmail.com wrote:

I see no difference in the above. (Yes, I checked multiple times that I was
using different executables).

Are you sure that you set core.filemode to false before testing?



yes.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-16 Thread Mark Levedahl

On 07/16/2013 05:36 PM, Ramsay Jones wrote:

Caveats:
1) I don't find any speed improvement of the current patch over the
previous one (the tests actually ran faster with the earlier patch,
though the difference was less than 1%).
2) I still question this whole approach, especially having this
non-POSIX compliant mode be the default. Running in this mode breaks
interoperability with Linux, but providing a Linux environment is the
*primary* goal of Cygwin.
Yes, I agree. Since I _always_ disable the Win32 stat functions (by
setting core.filemode by hand - yes it's a little annoying), I don't
get any benefit from the added complexity.

However, I don't use git on cygwin with *large* repositories. git.git
is pretty much as large as it gets. So, the difference in performance
for me amounts to something like 0.120s - 0.260s, which I can barely
notice.

Other people may not be so lucky ...

I would be happy if my original patch (removing the win32 stat funcs)
was applied, but others may not be. :-P

ATB,
Ramsay Jones

Cygwin 1.7 is very different than the earlier, no longer supported, and 
no longer available Cygwin variants in many ways, but stat is one of 
them. Cygwin 1.7 uses Windows ACLs to represent file permissions, and 
therefore gets the file permissions directly from the underlying OS 
calls. Earlier Cygwin versions (attempted to) overlay POSIX permissions 
on Windows systems using extended attributes and other means, and in 
many cases had to resort to opening the file and examining it to 
determine executability. This is not true in 1.7.


Therefore, your later patch would be expected to have much less benefit 
for 1.7 than for 1.5 (I don't detect *any* benefit on 1.7 when I set 
core.filemode=false). There are many choices, three are:


a) Remove the win32 stat funcs, eliminating all of the troublesome code 
paths and maintenance burden (your original patch).
b) Add your latest patch, with attendant complexity and maintenance 
burden, to support a version of Cygwin that is no longer available and 
was last updated over four years ago.
c) Like b, except make this triggered only by a CYGWIN_15 macro, 
limiting this to use by the legacy cygwin platform.


I strongly vote for a, could support c, but fear b is just going to keep 
us chasing down bugs. Especially so when we consider that this patch can 
only speed things up when core.filemode=false, which mode:

a) causes git to fail its test suite.
b) breaks compatibility with Linux
c) violates the primary goal of the Cygwin project, which is to provide 
a Linux environment on Windows.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Cygwin has trustable filemode

2013-07-16 Thread Mark Levedahl

On 07/16/2013 05:20 PM, Ramsay Jones wrote:

Mark Levedahl wrote:

The supported Cygwin distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Huh? How is it running that test? Does cygwin 1.7 somehow allow tabs in
filenames? For me, on cygwin 1.5, that test reports:

 $ ./t3300-funny-names.sh
 1..0 # SKIP Your filesystem does not allow tabs in filenames
 $
Cygwin 1.7 accesses the file system in a very different way than 
1.5/earlier, so handles funny names with alacrity.
  

The motivation for the original patch had more to do with windows people
using win32 text editors which set the executable bit inappropriately.
(see commit c869753e).

Since I use cygwin tools (vim), I don't have this problem. :-D
This is a perfect use for the pre-commit script. I've been doing this 
for years, changing line endings and executability based upon file type. 
This could no doubt also be handled by gitattributes now as well. 
(Almost) all windows editors are perfectly happy with \n line endings, 
and none I know of care about execute permissions. I strongly believe 
that making the file line ending mode and execute status conform to 
cross-platform standards is the right approach rather than ignoring 
these on Windows then making others on other platforms clean up the mess 
later.


Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Use compat/regex on Cygwin

2013-07-16 Thread Mark Levedahl

On 07/16/2013 05:41 PM, Ramsay Jones wrote:

Mark Levedahl wrote:

Cygwin's regex library does not pass git's tests, so don't use it. This
fixes failures in t4018 and t4034.

Hmm, these tests have always passed for me on cygwin. So, this is
presumably a regression in the new-lib regex library versions used
by cygwin 1.5 and cygwin 1.7.

ATB,
Ramsay Jones



Yes, cygwin 1.7 now uses the newlib regex functions, and those are not 
quite up to snuff. Another case for calling 1.5 a separate platform 
altogether.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] t9200 - Allow cvs version 1.12

2013-07-15 Thread Mark Levedahl
cvs v1.12 does not correctly handle cvs co -d $DIR, which is shorthand
for mkdir $DIR, cd $DIR, cvs co, cd -. So, use the latter form.

Also cvs v1.12 does not necessarily match cvs v1.11 in the format of
CVS/Entries, and this causes a false failure in subtest 14. Eliminate
checking CVS/Entries for this one test, but keep the test that the
created file exists and is checked out.

With these changes, all tests in t9200 pass on Cygwin using its default
cvs version 1.12.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 t/t9200-git-cvsexportcommit.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
index 3fb3368..17cb554 100755
--- a/t/t9200-git-cvsexportcommit.sh
+++ b/t/t9200-git-cvsexportcommit.sh
@@ -28,7 +28,8 @@ rm -rf $CVSROOT $CVSWORK
 
 cvs init 
 test -d $CVSROOT 
-cvs -Q co -d $CVSWORK . 
+mkdir -p $CVSWORK 
+(cd $CVSWORK  cvs -Q co .) 
 echo empty 
 git add empty 
 git commit -q -a -m Initial 2/dev/null ||
@@ -313,7 +314,6 @@ test_expect_success 'commit a file with leading spaces in 
the name' '
git commit -m Add a file with a leading space 
id=$(git rev-parse HEAD) 
git cvsexportcommit -w $CVSWORK -c $id 
-   check_entries $CVSWORK  
space/1.1/|DS/1.1/|attic_gremlin/1.3/|release-notes/1.2/ 
test_cmp $CVSWORK/ space  space
 
 '
-- 
1.8.3.2.0.63

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] t9200 - Allow cvs version 1.12

2013-07-15 Thread Mark Levedahl

On 07/15/2013 06:06 PM, Junio C Hamano wrote:

Mark Levedahl mleved...@gmail.com writes:


cvs v1.12 does not correctly handle cvs co -d $DIR, which is shorthand
for mkdir $DIR, cd $DIR, cvs co, cd -. So, use the latter form.

Hmph, I think I've been using 1.12.13 and without seeing such a
breakage.  Do you mean exactly v1.12, not v1.12.x series?

Hmm, good instincts. Cygwin includes 1.12.13 which is what I used. I 
downloaded the sources, rebuilt, everything works fine, so apparently 
the Cygwin provided cvs binary is corrupt. I apologize for the noise, 
will take this to the Cygwin list.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-15 Thread Mark Levedahl

On 07/15/2013 03:49 PM, Junio C Hamano wrote:

Mark Levedahl mleved...@gmail.com writes:


In order to limit the adverse effects caused by this implementation,
we provide a new fast stat interface, which allows us to use this
only for interactions with the index (i.e. the cached stat data).

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
using your prior patch (removing the Cygwin specific lstat entirely)
and get the same results with both, so this seems ok from me.

My comparison point was created by reverting your current patch from
pu, then reapplying your earlier patch on top, so the only difference
was which approach was used to address the stat functions.

Caveats:
1) I don't find any speed improvement of the current patch over the
previous one (the tests actually ran faster with the earlier patch,
though the difference was less than 1%).
2) I still question this whole approach, especially having this
non-POSIX compliant mode be the default. Running in this mode breaks
interoperability with Linux, but providing a Linux environment is the
*primary* goal of Cygwin.

Sounds like we are better off without this patch, and instead remove
the schizophrenic stat?  I do not have a strong opinion either
way, except that I tend to agree with your point 2) above.

In case my opinion is unclear, I think removal of the schizophrenic stat 
is the right approach. Speed is important, but not at the expense of 
correctness.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-15 Thread Mark Levedahl

On 07/15/2013 10:06 PM, Torsten Bögershausen wrote:

On 2013-07-15 21.49, Junio C Hamano wrote:

Mark Levedahl mleved...@gmail.com writes:


In order to limit the adverse effects caused by this implementation,
we provide a new fast stat interface, which allows us to use this
only for interactions with the index (i.e. the cached stat data).

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---

I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results
using your prior patch (removing the Cygwin specific lstat entirely)
and get the same results with both, so this seems ok from me.

My comparison point was created by reverting your current patch from
pu, then reapplying your earlier patch on top, so the only difference
was which approach was used to address the stat functions.

Caveats:
1) I don't find any speed improvement of the current patch over the
previous one (the tests actually ran faster with the earlier patch,
though the difference was less than 1%).

Hm, measuring the time for the test suite is one thing,
did you measure the time of git status with and without the patch?

(I don't have my test system at hand, so I can test in a few days/weeks)
Timing for 5 rounds of git status in the git project. First, with the 
current fast_lstat patches:

/usr/local/src/gitfor i in {1..5} ; do time git status  /dev/null ; done

real0m0.218s
user0m0.000s
sys 0m0.218s

real0m0.187s
user0m0.077s
sys 0m0.109s

real0m0.187s
user0m0.030s
sys 0m0.156s

real0m0.203s
user0m0.031s
sys 0m0.171s

real0m0.187s
user0m0.062s
sys 0m0.124s

Now, with Ramsay's original patch just removing the non-Posix stat 
functions:

/usr/local/src/gitfor i in {1..5} ; do time git status  /dev/null ; done

real0m0.218s
user0m0.046s
sys 0m0.171s

real0m0.187s
user0m0.015s
sys 0m0.171s

real0m0.187s
user0m0.015s
sys 0m0.171s

real0m0.187s
user0m0.047s
sys 0m0.140s

real0m0.187s
user0m0.031s
sys 0m0.156s


I see no difference in the above. (Yes, I checked multiple times that I 
was using different executables).

2) I still question this whole approach, especially having this
non-POSIX compliant mode be the default. Running in this mode breaks
interoperability with Linux, but providing a Linux environment is the
*primary* goal of Cygwin.

Sounds like we are better off without this patch, and instead remove
the schizophrenic stat?  I do not have a strong opinion either
way, except that I tend to agree with your point 2) above.

My understanding is that we want both:
Introduction of fast_lstat() as phase 1,
and the removal of the schizophrenic stat in compat/cygwin.c
as phase 2. (or do I missunderstand something ?)


And yes, phase 3:
The day we have a both reliable and fast
lstat() in cygwin, we can remove compat/cygwin.[ch]
If you know how to make Cygwin's stat faster while maintaining Posix 
semantics, please post a patch to the Cygwin list, they would *love* it.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-lib.sh - cygwin does not have usable FIFOs

2013-07-14 Thread Mark Levedahl

On 07/13/2013 08:57 PM, Jonathan Nieder wrote:
I'm not sure I follow. Are you saying Windows users would never want 
to access Subversion repositories? Thanks, Jonathan 
Quite the contrary. SVN and git both work on Windows without having 
POSIX FIFOs  - Windows does have FIFOS, but the semantics are different 
than POSIX and this is why Cygwin's do not work as needed. There is no 
problem having a subprocess with stdin and stdout redirected via 
anonymous pipes to the parent: this is how git runs sub commands and 
works fine. I'm just questioning why this same construct cannot be used 
for the test harness.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Cygwin has trustable filemode

2013-07-14 Thread Mark Levedahl
The supported Cygwin distribution on supported Windows versions provides
complete support for POSIX filemodes, so enable this by default. git as
distributed by the Cygwin project is configured this way.

This fixes one testsuite failure:
t3300 test 17 (diff-index -M -p with mode change quotes funny filename)

Historical notes: Earlier versions of Cygwin (version 1.5 and prior) had 
various methods for supporting posix file modes on different file systems, 
often using extended attributes, and this support was optional.  Such 
versions of Cygwin are not available on any public mirror and are not 
supported by the Cygwin project. The currently available Cygwin supports 
POSIX file modes without exception - this is not an optional 
configuration. The support does depend upon the underlying file system 
(neither Linux nor Cygwin can set an execute bit on a FAT file system as 
FAT has no such support), but as this is no different than Linux, the
default should not treat Cygwin differently than Linux.  

Users who desire the non-POSIX mode of operation must explicitly set 
core.filemode=False, accepting non-interoperability with Linux.  

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 config.mak.uname | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 7ac541e..779d06a 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -163,7 +163,6 @@ ifeq ($(uname_O),Cygwin)
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
-   NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 1/1] cygwin: Add fast_lstat() and fast_fstat() functions

2013-07-14 Thread Mark Levedahl

On 07/10/2013 04:23 PM, Ramsay Jones wrote:

Commit adbc0b6b (cygwin: Use native Win32 API for stat, 30-09-2008)
added a Win32 specific implementation of the stat functions. In order
to handle absolute paths, cygwin mount points and symbolic links, this
implementation may fall back on the standard cygwin l/stat() functions.
Also, the choice of cygwin or Win32 functions is made lazily (by the
first call(s) to l/stat) based on the state of some config variables.

Unfortunately, this schizophrenic stat implementation has been the
source of many problems ever since. For example, see commits 7faee6b8,
79748439, 452993c2, 085479e7, b8a97333, 924aaf3e, 05bab3ea and 0117c2f0.

In order to limit the adverse effects caused by this implementation,
we provide a new fast stat interface, which allows us to use this
only for interactions with the index (i.e. the cached stat data).

Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
---


I've tested this on Cygwin 1.7 on WIndows 7 , comparing to the results 
using your prior patch (removing the Cygwin specific lstat entirely) and 
get the same results with both, so this seems ok from me.


My comparison point was created by reverting your current patch from pu, 
then reapplying your earlier patch on top, so the only difference was 
which approach was used to address the stat functions.


Caveats:
1) I don't find any speed improvement of the current patch over the 
previous one (the tests actually ran faster with the earlier patch, 
though the difference was less than 1%).
2) I still question this whole approach, especially having this 
non-POSIX compliant mode be the default. Running in this mode breaks 
interoperability with Linux, but providing a Linux environment is the 
*primary* goal of Cygwin.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Cygwin, git and x: directory

2013-07-12 Thread Mark Levedahl

On 07/12/2013 08:42 AM, Mikko Rapeli wrote:

(please Cc: me in replies, not subscribed to the lists)

Hi Cygwin and git developers,

Does following scenario show signs of bugs in Cygwin and/or git?

# setup git repo
$ cd /tmp
$ mkdir foo  cd foo
$ git init

# create x: directory
$ mkdir x:
$ ls
x:



I would report this on the Cygwin list, not here. Any use of dos file 
paths using a Cygwin tool is not recommended, officially only POSIX 
paths are supported. If cygwin's Cmake is generating dos style paths, 
that is a bug that needs reporting to the Cygwin list. Also, I'm not 
sure how the developers will react to mishandling a directory named x:, 
but the behaviour you see is a limitation of the Cygwin platform, not of 
git.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-lib.sh - cygwin does not have usable FIFOs

2013-07-08 Thread Mark Levedahl

On 07/06/2013 08:55 PM, Jonathan Nieder wrote:

Mark Levedahl wrote:


Do not use FIFOs on cygwin, they do not work. Cygwin includes
coreutils, so has mkfifo, and that command does something. However,
the resultant named pipe is known (on the Cygwin mailing list at
least) to not work correctly.

Hm.  How would you recommend going about writing a script that takes
output from a command, transforms it, and then feeds it back into
that command's input?  Are sockets a more reliable way to do this kind
of IPC on Cygwin?

See reinit_git and try_dump from t9010-svn-fe.sh for context.

Thanks,
Jonathan



On the one hand, sockets work fine on cygwin so that path would probably 
work.


However, I don't understand why git would need to consume its own output 
- If named pipes are really needed to use git-svn because git-svn 
depends upon git feeding the same git process, then that package should 
not be available on cygwin or any other platform that does not support 
fifos. If not, then I don't think the test suite should require fifos or 
any other construct with the same git process feeding itself either, it 
just blurs the line about what is actually being tested.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] test-lib.sh - cygwin does not have usable FIFOs

2013-07-04 Thread Mark Levedahl
Do not use FIFOs on cygwin, they do not work. Cygwin includes
coreutils, so has mkfifo, and that command does something. However,
the resultant named pipe is known (on the Cygwin mailing list at
least) to not work correctly.

This disables PIPE for Cygwin, allowing t0008.sh to complete (all other
tests in that file work correctly).

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 t/test-lib.sh | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9753641..2d63307 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -885,7 +885,14 @@ test_i18ngrep () {
 
 test_lazy_prereq PIPE '
# test whether the filesystem supports FIFOs
-   rm -f testfifo  mkfifo testfifo
+   case $(uname -s) in
+   CYGWIN*)
+   false
+   ;;
+   *)
+   rm -f testfifo  mkfifo testfifo
+   ;;
+   esac
 '
 
 test_lazy_prereq SYMLINKS '
-- 
1.8.3.2.0.13

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions

2013-06-27 Thread Mark Levedahl

On 06/27/2013 06:58 PM, Ramsay Jones wrote:


This is why I tried the cygwin: Remove the Win32 l/stat()
functions patch first; I think this is the correct approach
to fixing this problem (and similar *future* problems).

I adamantly agree.

However, since that is no longer an option, on performance
grounds, I have other ideas I want to try. (see later email)


Correctness first, speed later.

1) Keep the patch to remove the buggy and unreliable stat / lstat.
2) We fix the remaining test failures.
3) With the test suite passing, stat optimization(s) that cause no 
failures / regressions can be accepted.


With the msys/mingw git available for years now, there really is not a 
reason to make Cygwin's git violate the Posix expectations of that 
platform. msys makes no such promises, so is the right tool for those on 
Windows who just want git as fast as possible on Windows (still 
slow) and don't care about file modes, softlinks, etc.


I'm keeping your patch in my tree.

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] CYGWIN: Use a TCP socket for pipe()

2013-06-27 Thread Mark Levedahl

On 06/27/2013 01:38 PM, Junio C Hamano wrote:

Torsten Bögershausen tbo...@web.de writes:


Work around issues that git hangs when doing fetch or pull under
various protocols under CYGWIN.

Replace pipe() with a socket connection using a TCP/IP.
Introduce a new function socket_pipe() in compat/socket_pipe.c

Sounds like sweeping the real problem, whatever it is, under rug.
Is it that we are assuming a pipe buffer that is sufficiently large
and expecting a write that we deem to be small enough not to block,
causing a deadlock on a platform with very small pipe buffer, or
something?



There were issues in early v1.7 Cygwin release for overlapping I/O such 
that the pipe was sometimes terminated early resulting in data loss. If 
the pipe implementation in Cygwin is still a problem a good test case 
sent to the Cygwin developers would be the right approach rather than a 
one-off patch in git to try to work around a current platform bug.


Note - I do not see random hangs with the stat/lstat hack removed, 
rather the sole test suite hang I see is repeatable in t0008.sh. So, if 
the patch remains, we may be able to run this remaining hang to ground.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions

2013-06-26 Thread Mark Levedahl

On 06/26/2013 10:19 AM, Torsten Bögershausen wrote:

On 2013-06-25 23.18, Junio C Hamano wrote:

Johannes Sixt j...@kdbg.org writes:


Some context: This is about a patch by Ramsay that removes the
schizophrenic lstat hack for Cygwin. Junio, can you please queue that
patch in pu?

Sure.  Thanks.

First of all,
thanks for the work.

Here some benchmark results,
(The test run of the test suite did the same amout of time).

But:
git status -uno in real life takes double the time,
git 1.8.3 compared against pu with the vanilla l/stat

 1 second -  2 seconds on linux kernel

0.2 seconds - 0.4 seconds on git.git

Do we have any known problems with the current implementation ?
Does speed matter ?

One vote to keep the special cygwin functions.
(And have a look how to improve the core.filemode)

/Torsten


There have been threads on the cygwin mailing lists for at least a 
decade looking to speed up cygwin's posix stat / lstat (and fork). If 
improvement were merely difficult, it would have been done long ago. As 
git cares about things like execute bits, file / repository permissions, 
and soft links, whatever stat / lstat git uses needs to fully support 
those under cygwin, either by using what cygwin provides or providing a 
complete replacement. Note my other reply - with Ramsay's patch I can 
complete the test suite (except for t0008.sh that has a known hang) 
while without it I find the test suite randomly (unrepeatable) hangs in 
many tests. So, this stat/lstat replacement is at least implicated in 
current troubles.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Problems with Windows, Was: What's cooking in git.git (May 2013, #01; Fri, 3)

2013-05-07 Thread Mark Levedahl

On 05/07/2013 10:27 AM, Torsten Bögershausen wrote:

On 2013-05-04 01.14, Junio C Hamano wrote:


  Cygwin portability; both were reviewed by Jonathan, and the tip one
  seems to want a bit further explanation.  Needs positive report
  from Cygwin 1.7 users who have been on 1.7 to make sure it does not
  regress for them.


I was trying to verify that cygwin 1.7 is still Ok, but got puzzled.

Running the test suite under cygwin doesn't seem to work any more (?):

Scenario 1:
The PC is running alone, and goes into the screen saver.
Pressing CTRL-ALT-DEL didn't get any effect.

Scenario 2:
The PC didn't react any more, when the test suite was run in background.
In 3 or 4 cases the PC needed to be reboot hardly.

Using the commits before and after this change makes the test suite hang
as well at some point, then it hangs somewhere at TC 3000--4000.

Scenario 4:
The I disabled the screensaver, upgdated cygwin,
  and went back to an older commit:
The latest run from commit 52d63e70, April 28,
hangs in TC 5500, ok 26 clone shallow object count.

I can see 2 times
git.exe pull --depth 4 ..A

Scenario 5:
The run of today 1.8.3-rc1, hangs in t5510,
some git.exe are running fetch. (or pull)


It seems as if some process/exes are not terminated
in the way it should be.

I will try on a different machine,
comments are wellcome
/Torsten



I have run into very similar problems trying to test these patches, so I 
declined to reply thinking someone else might have better or at least 
explicable results. I am able to build git on cygwin 1.7 using the 
proposed patches, it seems to work, but I've run into strange problems 
such as the main git repo becoming corrupted, no idea how or why.


Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS

2013-02-25 Thread Mark Levedahl

On 02/25/2013 01:44 AM, Junio C Hamano wrote:
I was in find leftover bits mode today and found this thread 
hanging. Has anything come out of this thread, or there is nothing to 
improve in this area? 


The patch passed my simple tests (build, run a few commands), but I 
didn't get around to a full test. And of course, I am testing on current 
Cygwin where git compiles and runs correctly anyway.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] mingw: rename WIN32 cpp macro to NATIVE_WINDOWS

2013-01-26 Thread Mark Levedahl

On 01/25/2013 08:03 PM, Jonathan Nieder wrote:

diff --git a/abspath.c b/abspath.c
index 40cdc462..c7d5458e 100644
--- a/abspath.c
+++ b/abspath.c
@@ -216,7 +216,7 @@ const char *absolute_path(const char *path)
  const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
  {
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WINDOWS_NATIVE
if (!pfx_len || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/compat/terminal.c b/compat/terminal.c
index 9b5e3d1b..136e4a74 100644


Maybe WINDOWS_NATIVE should be defined in config.mak.uname?

Otherwise, I tested the patch and it does build / run on Cygwin, but I 
cannot run a test suite until next week. I am concerned about subtle 
changes due to the  various WIN32 tests that were not guarded by 
__CYGWIN__ before, haven't stared at the code enough to see if there 
could be an issue.


Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Version 1.8.1 does not compile on Cygwin 1.7.14

2013-01-25 Thread Mark Levedahl

On 01/22/2013 01:31 PM, Ramsay Jones wrote:
include order. ;-) As I have mentioned here before, the claim that 
WIN32 is not defined on cygwin is simply nonsense - it depends on 
if/when certain header files are included. For example, *as soon as* 
you include windows.h (and, I suspect, many other win32 headers) 
then defined(WIN32) is true. Note that commit 380a4d92 (Update 
cygwin.c for new mingw-64 win32 api headers, 11-11-2012) swaps the 
include order for the win32.h and git-compat-util.h header files. [I 
don't know the details, Mark didn't elaborate, but it is clearly an 
include order problem on cygwin 1.7.x :-D ] This causes compilation 
errors on cygwin 1.5.x, exactly because win32.h includes windows.h, 
which defines WIN32, which then leads to git-compat-util.h including 
winsock2.h.

  #if defined(WIN32)  defined(__CYGWIN__)
  # undef WIN32
  #endif


Cygwin and Windows should be treated as completely separate platforms: 
if __CYGWIN__ is defined, do one thing, if not, go ahead and check 
WIN32, but the WIN32 macro should never be tested once we know the 
platform is CYGWIN - these really are different platforms (if you are 
unsure of this, consider that Cygwin includes a cross-compiler to target 
native Win32 as the Cygwin maintainers recognized the platforms are 
different).


Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Version 1.8.1 does not compile on Cygwin 1.7.14

2013-01-06 Thread Mark Levedahl

On 01/06/2013 04:57 AM, Jonathan Nieder wrote:

Torsten Bögershausen wrote:


The short version:
Cygwin versions  1.7.1 up to 1.7.16 use the same header files as cygwin 1.5

[...]

I don't know if we want to improve the Makefile to enable
CYGWIN_V15_WIN32API = YesPlease
for cygwin versions 1.7.1 .. 1.7.16 (which are outdated)



You are conflating the cygwin dll version with the win32 api version. 
These are independent packages (just as the kernel and glibc packages 
are independent on linux) and do not share a version number. However, 
the newer win32api is provided only for the current cygwin release 
series, which can be reliably identified by having dll version 1.7.x, 
while the older frozen releases (dll versions 1.6.x from redhat, 1.5.x 
open source) still have the older api as no updates are being made for 
the legacy version(s).


Cygwin does not version the win32api in any useful way: the package 
names changed completely, for instance, and there is no macro defined 
from the header files to indicate a version number. Also, there is no 
supported way to now install the older version: the only supported 
configuration is with the *current* win32api: multiple packages depend 
by name on the current win32api package, so the installer will insist 
upon its installation.


So the solution is to update the cygwin installation. Really. If you 
don't believe me, try asking on the cygwin mailing list. They only 
support the current releases, not obsolete packages, and the older 
win32api is explicitly obsolete.


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Version 1.8.1 does not compile on Cygwin 1.7.14

2013-01-06 Thread Mark Levedahl

On 01/06/2013 02:54 PM, Junio C Hamano wrote:

Jonathan Nieder jrnie...@gmail.com writes:


Mark Levedahl wrote:


  However, the newer
win32api is provided only for the current cygwin release series, which can
be reliably identified by having dll version 1.7.x, while the older frozen
releases (dll versions 1.6.x from redhat, 1.5.x open source) still have the
older api as no updates are being made for the legacy version(s).

Ah.  That makes sense, thanks.

(For the future, if we wanted to diagnose an out-of-date win32api and
print a helpful message, I guess cygcheck would be the command to use.)

Hmph, so we might see somebody who cares about Cygwin to come up
with a solution based on cygcheck (not on uname) to update this
part, perhaps on top of Peff's split default settings based on
uname into separate file patch?

If I understood what Mark and Torsten wrote correctly, you will have
the new win32api if you install 1.7.17 (or newer) from scratch, but
if you are on older 1.7.x then you can update the win32api part as a
package update (as opposed to the whole-system upgrade).  A test
based on uname -r cannot notice that an older 1.7.x (say 1.7.14)
installation has a newer win32api because the user updated it from
the package (hence the user should not define CYGWIN_V15_WIN32API).

Am I on the same page as you guys, or am I still behind?

In the meantime, perhaps we would need something like this?


diff --git a/Makefile b/Makefile
index 8e225ca..b45b06d 100644
--- a/Makefile
+++ b/Makefile
@@ -281,6 +281,9 @@ all::
  #
  # Define NO_REGEX if you have no or inferior regex support in your C library.
  #
+# Define CYGWIN_V15_WIN32API if your Cygwin uses win32api dll older than
+# 1.7.x (this typically is true on Cygwin older than 1.7.17)
+#
  # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
  # user.
  #

Looking at a current setup.ini, the obsolete win32 api is a single 
package w32api with last version 3.17-2, and is now replaced by the 
new win32 api is in two packages, w32api-headers + w32api-runtime, 
both at version 3.0b_svn5496-1. If setup.exe updated an older 
installation of w32api, the old package is not deleted, but replaced by 
a special empty package with (as of today) version -1. Note that 
all of this could change at any time. Also, note that the new w32api 
packages have version numbers that are lower than the older obsoleted 
version.


Running cygcheck -c w32api w32api-headers w32api-runtime on one 
machine gives


Cygwin Package Information
Package  VersionStatus
w32api   -1 OK
w32api-headers   3.0b_svn5496-1 OK
w32api-runtime   3.0b_svn5496-1 OK

So now, what do folks propose checking for?
a) w32api is installed? Nope - the package is not removed, it was 
updated to a special empty version to delete its former contents, but a 
new fresh installation won't have this.

b) w32api-headers is installed? Nope - what happens on the next repackaging?
c) w32api version is -1? Maybe, but that number could change.
etc.

There is no documented, reliable, future-proof, method of determining 
the installed w32api version on Cygwin. There are many things that can 
be done that will work frequently, except when they won't. I really 
think the only sane thing is to follow the guidance of the Cygwin 
developers: the only supported configuration is that which the current 
setup.exe produces, and in the case of problems, if the installation is 
not up to date then updating is the first required action.


So, in the makefile, you might add:

+# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
+# using the current w32api packages. But, the recommended approach is to
+# update your installation if compilation errors occur.
+#

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Version 1.8.1 does not compile on Cygwin 1.7.14

2013-01-06 Thread Mark Levedahl

On 01/06/2013 03:51 PM, Torsten Bögershausen wrote:
Hm, I haven't understood the connection between the dll (cygwin1.dll 
?) which is used in runtime, and the header files which are used when 
compiling. Are they updated at the same time when updating from 1.7.16 
to 1.7.17 ? Until I updated my cygwin 1.7 (following Marks 
recommendation) this did the trick for me: +ifeq ($(shell grep mingw 
/usr/include/w32api/winsock2.h //dev/null 2/dev/null  echo y),y) + 
CYGWIN_V15_WIN32API=YesPlease +endif As an alternative, would this be 
easier to read?

+# Define CYGWIN_V15_WIN32API for Cygwin versions up to 1.7.16





The cygwin distribution has a very large number of packages, each with 
its own unique version number and update rhythm, just as in any linux 
distro. There is no cygwin version, just a version for each individual 
package. So, Cygwin version 1.7.16 is really nonsensical: there is 
only cygwin.dll version 1.7.16.  What folks are noticing is a 
coincidence in the time when the cygwin dll package updated and when the 
old w32api was obsoleted. uname -r reports the cygwin dll version, not 
the version of any other package. Note that the cygwin api is stable, 
meaning a package compiled against the 1.7.1 dll will still run against 
the current one: updating the cygwin dll does not require other packages 
to update.


The only hard linkage here is that the Cygwin developers are maintaining 
a legacy cygwin version (v1.5.x) as the newer dll series (v.1.7.x) 
dropped support for all Windows versions predating (I think) WinXP. So, 
someone on an old Windows version must use the legacy cygwin version 
which has not been updated since the first v1.7 dll was released, nor 
are there any plans by the developers to ever update the v1.5 packages. 
Cygwin 1.5 lives in a separate distribution repository, with packages 
frozen in time as of the last updates prior to going to v1.7 (packages 
compiled against v1.7 will not run on v.1.5).


So, encountering a v1.5.x dll is a guarantee of using the older w32api 
shared with the mingw project, rather than the current one now 
maintained by the mingw64 project. However, a cygwin with any v1.7.x dll 
could in theory have either w32api installed, or in theory yet another 
newer one we don't know about yet. Unless and until the w32api 
establishes a version number (independent of the Windows API version), 
we have nothing reliable to use.


Therefore, if using the v1.7 series, *update*

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Version 1.8.1 does not compile on Cygwin 1.7.14

2013-01-06 Thread Mark Levedahl

On 01/06/2013 04:35 PM, Junio C Hamano wrote:

Thanks; so perhaps you can give me an OK to forge your S-o-b to the
following?

-- 8 --
From: Mark Levedahl mleved...@gmail.com
Date: Sun, 6 Jan 2013 11:56:33 -0800
Subject: [PATCH] Makefile: add comment on CYGWIN_V15_WIN32API

There is no documented, reliable, and future-proof method to
determine the installed w32api version on Cygwin. There are many
things that can be done that will work frequently, except when they
won't.

The only sane thing is to follow the guidance of the Cygwin
developers: the only supported configuration is that which the
current setup.exe produces, and in the case of problems, if the
installation is not up to date then updating is the first required
action.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
  Makefile | 4 
  1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 4d47af5..52e298a 100644
--- a/Makefile
+++ b/Makefile
@@ -273,6 +273,10 @@ all::
  #
  # Define NO_REGEX if you have no or inferior regex support in your C library.
  #
+# Define CYGWIN_V15_WIN32API if you are using Cygwin v1.7.x but are not
+# using the current w32api packages. The recommended approach, however,
+# is to update your installation if compilation errors occur.
+#
  # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
  # user.
  #

Absolutely, that is ok by me.

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: sys/param.h

2012-12-19 Thread Mark Levedahl

On 12/19/2012 02:59 AM, Erik Faye-Lund wrote:

On Tue, Dec 18, 2012 at 6:01 PM, Junio C Hamano gits...@pobox.com wrote:

Johannes Sixt j.s...@viscovery.net writes:


Junio C Hamano wrote:

It could turn out that we may be able to get rid of sys/param.h
altogether, but one step at a time.  Inputs from people on minority
platforms are very much appreciated---does your platform build fine
when the inclusion of the file is removed from git-compat-util.h?


cygwin is fine with that removed.

Mark

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] USE CGYWIN_V15_WIN32API as macro to select api for cygwin

2012-11-18 Thread Mark Levedahl
The previous macro was confusing to some, and did not include cygwin in
its name. The updated name more clearly expresses a choice of the
win32api implementation that shipped with version 1.5 of cygwin.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 Makefile| 6 +++---
 compat/cygwin.c | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index cf0ecde..9731c85 100644
--- a/Makefile
+++ b/Makefile
@@ -1091,7 +1091,7 @@ ifeq ($(uname_O),Cygwin)
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
OLD_ICONV = UnfortunatelyYes
-   V15_MINGW_HEADERS = YesPlease
+   CYGWIN_V15_WIN32API = YesPlease
endif
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
@@ -1906,8 +1906,8 @@ ifdef NO_REGEX
COMPAT_CFLAGS += -Icompat/regex
COMPAT_OBJS += compat/regex/regex.o
 endif
-ifdef V15_MINGW_HEADERS
-   COMPAT_CFLAGS += -DV15_MINGW_HEADERS
+ifdef CYGWIN_V15_WIN32API
+   COMPAT_CFLAGS += -DCYGWIN_V15_WIN32API
 endif
 
 ifdef USE_NED_ALLOCATOR
diff --git a/compat/cygwin.c b/compat/cygwin.c
index 59d86e4..5428858 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,5 +1,5 @@
 #define WIN32_LEAN_AND_MEAN
-#ifdef V15_MINGW_HEADERS
+#ifdef CYGWIN_V15_WIN32API
 #include ../git-compat-util.h
 #include win32.h
 #else
-- 
1.8.0.0.0.14

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Rename V15_MINGW_HEADERS into CYGWIN_OLD_WINSOCK_HEADERS

2012-11-17 Thread Mark Levedahl

On 11/17/2012 02:09 AM, Torsten Bögershausen wrote:

See commit 380a4d927bff693c42fc6b22c3547bdcaac4bdc3:
Update cygwin.c for new mingw-64 win32 api headers
Cygwin up to 1.7.16 uses some header file from the WINE project
Cygwin 1.7.17 uses some header file from the mingw-64 project
As the old cygwin (like 1.5) never used mingw,
the name V15_MINGW_HEADERS is confusing.
Rename it into CYGWIN_OLD_WINSOCK_HEADERS


diff --git a/Makefile b/Makefile
index c3edf8c..c2ea735 100644
--- a/Makefile
+++ b/Makefile
@@ -1089,7 +1089,7 @@ ifeq ($(uname_O),Cygwin)
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
OLD_ICONV = UnfortunatelyYes
-   V15_MINGW_HEADERS = YesPlease
+   CYGWIN_OLD_WINSOCK_HEADERS = YesPlease
  
WINSOCK is certainly a part of the win32 api implementation, but it is 
is the entire win32api that changed, not just the tiny bit dealing with 
sockets.
Basically, WINDOWS.h, and everything it includes, and all of the dlls it 
touches, and the .o files, changed. Calling it OLD is not helpful, 
what happens in the future with the next change? The only version info 
we really have is the dll version. We are switching between the win32 
api implementation shipped with cygwin dll version 1.5.x and the one 
that is now current. And, the shift is not tied to any particular cygwin 
1.7.x dll version either (there are no cross dependencies between the 
win32api implementation and any particular dll in the 1.7.x series, just 
a coincidence in time as to what packages got updated when). So my 
suggestion in the bike shedding category is to


s/V15_MINGW_HEADERS/CYGWIN_V15_WIN32API/

/end of bike shedding.

If this is really worth a second patch, I'll be happy to send one :^)

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)

2012-11-15 Thread Mark Levedahl

On 11/15/2012 02:35 PM, Torsten Bögershausen wrote:

On 11/15/2012 08:05 PM, Ramsay Jones wrote:


Did the cygwin project not bump an api version number somewhere?

ATB,
Ramsay Jones

Ramsay,
you can run uname -r to see the version number.

I myself haven't fully understood all the consequences,
somewhere between 1.7.7 and 1.7.17 the include files had been changed.

If this has consequences for using e.g. winsock2.dll, I want to know 
myself ;-)


/Torsten


uname -r gives the version of the dll, not of any particular package, 
and all of the various components are versioned independently. The 
win32api is spread across several packages, and the recent update 
changed the names, There is no api number advertised. You could check 
the names of currently installed packages if you assume those names 
won't change, but any such method is going to be fragile (and very 
unsupported).


Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)

2012-11-14 Thread Mark Levedahl

On 11/14/2012 07:16 PM, Jeff King wrote:

On Wed, Nov 14, 2012 at 10:13:28PM +0100, Torsten Bögershausen wrote:
b) Autodetection:
   (Just loud thinking), running
$grep mingw /usr/include/w32api/winsock2.h
  * This file is part of the mingw-w64 runtime package.
#include _mingw_unicode.h

on cygwin 1.7.17 indicates that we can use grep in the Makefile to
autodetect the mingw headers
Hmm. Can we rely on the /usr/include bit, though?

I assume a test-compile would be sufficient, but currently we do not do
anything more magic than uname in the Makefile itself to determine
defaults.  Maybe it would be better to do the detection in the configure
script? And then eventually flip the default in the Makefile once
sufficient time has passed for most people to want the new format (which
would not be necessary for people using autoconf, but would help people
who do not).

-Peff



Cygwin changed the win32api implementation, and the old is not just no 
longer supported for the current release series, but virtually 
impossible to even install (several new packages are now installed, the 
old package is in the obsolete category, i.e., not available). The 
older cygwin 1.5 dll + utilities can be installed afresh, so that is why 
I set up to switch based upon dll version - the proposed test(s) and 
configuration would be to have git maintain compatibility with an 
unsupported Cygwin configuration. I just don't think this is worth the 
maintenance burden, but of course I am not the maintainer, just 
expressing my opinion.


I have no trouble renaming the macro to whatever seems to clarify things.

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Nov 2012, #03; Tue, 13)

2012-11-13 Thread Mark Levedahl

On 11/13/2012 03:45 PM, Torsten Bögershausen wrote:

* ml/cygwin-mingw-headers (2012-11-12) 1 commit
  - Update cygwin.c for new mingw-64 win32 api headers

  Make git work on newer cygwin.

  Will merge to 'next'.

(Sorry for late answer, I managed to test the original patch minutes before 
Peff merged it to pu)
(And thanks for maintaining git)

Is everybody using cygwin happy with this?

I managed to compile on a fresh installed cygwin,
but failed to compile under 1.7.7, see below.
Is there a way we can achieve to compile git both under old and new cygwin 
1.7 ?
Or is this not worth the effort?
/Torsten



I found no version info defined that could be used to automatically 
switch between the old and current headers. You can always


make V15_MINGW_HEADERS=1 ...

to force using the old set if you do not wish to update your installation.

Mark
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Update cygwin.c for new mingw-64 win32 api headers

2012-11-11 Thread Mark Levedahl
The cygwin project recently switched to a new implementation of the
windows api, now using header files from the mingw-64 project. These
new header files are incompatible with the way cygwin.c included the
old headers: cygwin.c can be compiled using the new or the older (mingw)
headers, but different files must be included in different order for each
to work. The new headers are in use only for the current release series
(based upon the v1.7.x dll version). The previous release series using
the v1.5 dll is kept available but unmaintained for use on older versions
of Windows. So, patch cygwin.c to use the new include ordering only if
the dll version is 1.7 or higher.

Signed-off-by: Mark Levedahl mleved...@gmail.com
---
 Makefile| 4 
 compat/cygwin.c | 7 +++
 2 files changed, 11 insertions(+)

diff --git a/Makefile b/Makefile
index f69979e..1cc5d96 100644
--- a/Makefile
+++ b/Makefile
@@ -1082,6 +1082,7 @@ ifeq ($(uname_O),Cygwin)
NO_SYMLINK_HEAD = YesPlease
NO_IPV6 = YesPlease
OLD_ICONV = UnfortunatelyYes
+   V15_MINGW_HEADERS = YesPlease
endif
NO_THREAD_SAFE_PREAD = YesPlease
NEEDS_LIBICONV = YesPlease
@@ -1889,6 +1890,9 @@ ifdef NO_REGEX
COMPAT_CFLAGS += -Icompat/regex
COMPAT_OBJS += compat/regex/regex.o
 endif
+ifdef V15_MINGW_HEADERS
+   COMPAT_CFLAGS += -DV15_MINGW_HEADERS
+endif
 
 ifdef USE_NED_ALLOCATOR
COMPAT_CFLAGS += -Icompat/nedmalloc
diff --git a/compat/cygwin.c b/compat/cygwin.c
index dfe9b30..59d86e4 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,6 +1,13 @@
 #define WIN32_LEAN_AND_MEAN
+#ifdef V15_MINGW_HEADERS
 #include ../git-compat-util.h
 #include win32.h
+#else
+#include sys/stat.h
+#include sys/errno.h
+#include win32.h
+#include ../git-compat-util.h
+#endif
 #include ../cache.h /* to read configuration */
 
 static inline void filetime_to_timespec(const FILETIME *ft, struct timespec 
*ts)
-- 
1.8.0.0.0.14

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html