Re: [PATCH] config.mak.uname: Cygwin: Use renames for creation
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
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
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
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
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
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
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
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
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
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
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
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
$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)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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)
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
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
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
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
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
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
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
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
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
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
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)
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)
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)
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
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