two minor patches to make git play more nicely in the mingw environment #1
Conversation
…n msys environment
…ith path environment variable
|
Sorry, I did not realize that the special github.com mail address I replied to would not publish this mail here: Hi Brian, On Fri, 16 Sep 2011, brian wrote:
It works for me, and besides, you change the way errors are handled. Now Besides, you sneak in a change of "abbrevguard" to "abbrev". How come?
The problem seems to be that Windows' environment ignores case? Then there BTW I do not think it is appropriate for us to discuss these things in Ciao, |
|
__git_ps1 works for you? I'm surprised to hear that, as it is a 100% replicable failure for me on every version of MSYSGIT. This is what I'm trying to do: http://blog.jasonmeridth.com/2010/05/22/git-ps1.html It won't work on msys unless I modify the script, as bash in MSYS chokes on the "done < < (..." syntax. I'll look at finding a better fix that doesn't modify the error handling. The change of "abbrevguard" to "abbrev" was a mistake, I had a older version of git_completion, and missed that. I am pretty sure that Windows environment variable names are, in fact, case insensitive. In that case, the suggested fix for submodule.sh seems like a good workaround. |
|
I have to say that I find it a bit funny to discuss Open Source as closed as on this particular thread in which exactly two people participate, one of which cannot reproduce the issue. But hey, I'm not against it, at least I have the privilege to be invited to the show! Oh, and it is quite nice for Windows to have an environment where the variable names are, in fact, case sensitive. But in fact, I hate patches breaking Windows support as much as I don't like patches breaking non-Windows setups. I, for one, use the exact same source as Git for Windows to build and test on Linux. To make sure that we don't have any changes that deviate in functionality from how Git is supposed to operate, and also to support my daily workflow. So I am afraid that unless I get convinced that the proposed change is portable (in my setup, "path" and "paTH" and "PATH" are different environment variables) I will have to refrain from pulling those changes into our mainline branch. Other people in the msysGit project might be more willing to accept such a patch, or at least work on it until it is not breaking non-Windows setups, but as I might have mentioned earlier, this thread is rather exclusive, and they are not as blessed as I am! :-) |
|
I'm sorry you aren't comfortable using GitHub to discuss a project that you mirrored on GitHub. If you plan to keep this mirror active, you may want to familiarize yourself with the pull requests (http://help.github.com/send-pull-requests/). I'm not sure I understand how a comment thread that is visible to the entire internet (https://github.com/msysgit/git/pulls) is more closed than a subscriber only mailing list, but whatever you say. I'm all for using the same git sources on Windows and Linux, but in my experience, two pieces of fuctionality are broken on Windows:
I don't have the skill to get those fixed in the msys environment, so I came up with workarounds for them in the scripts that implement the functionality for git. I figured those workarounds might be helpful to others who work with submodules or use __git_ps1, so I submitted a pull request. Sorry to offend. |
|
You misunderstood: I am quite comfortable with the GitHub interface, don't you doubt that. But the msysGit project does not consist of me alone, and I have a slight suspicion that some people would regard the two of us discussing patches here and them not knowing about it would be perceived as secretive when the recommended way (and advertised as such) is the mailing list, which is not subscribers-only. It is just moderated, and if you think that this is not necessary, I am happy to forward you the 10+ spam mails that the list receives per day (yes, way more than non-spam mails, way more). Now, as to your problem, maybe I still do not understand your setup, because here, it works perfectly, with 3 different Windows installations and more Linux and MacOSX installations. I have no problem on those. Furthermore, on all of those setups I use submodules heavily. And last time I checked, they were working exactly as I wanted them to. Now, maybe I missed it, but I think that
Please understand that I will try not to apply a patch that fixes things for you but breaks existing setups. |
|
I have the problem with The patch junkshow/git@d48d298 works for me. |
|
Sure, that patch works around the "cannot make pipe" issue. But it does change the failure mode, as I mentioned earlier. Plus, that commit never has been cleaned up regarding the abbrevguard issue, so I am hesitant to take the patch since I fear I would be stuck with further modifications as people see issues in that code. |
|
I think this issue should be taken over the the git list, no? Do you see any solution (not workaround) for this issue ? |
|
I would be all for moving this to the list. A workaround could be to use a construct similar to this: output="$(first command)" |
|
This has been merged into msysGit from upstream. |
|
Thank you all for getting this issue resolved, it is very much appreciated. I will study and try to do a better job with the next one! Thanks again! |
entry_count is used in update_one() for two purposes: 1. to skip through the number of processed entries in in-memory index 2. to record the number of entries this cache-tree covers on disk Unfortunately when CE_REMOVE is present these numbers are not the same because CE_REMOVE entries are automatically removed before writing to disk but entry_count is not adjusted and still counts CE_REMOVE entries. Separate the two use cases into two different variables. #1 is taken care by the new field count in struct cache_tree_sub and entry_count is prepared for #2. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The DWIM mode of checkout allows you to run "git checkout foo" when there is no existing local ref or path called "foo", and there is exactly _one_ remote with a remote-tracking branch called "foo". Git will automatically create a new local branch called "foo" using the remote-tracking "foo" as its starting point and configured upstream. For example, consider the following unconventional (but perfectly valid) remote setup: [remote "origin"] fetch = refs/heads/*:refs/remotes/origin/* [remote "frotz"] fetch = refs/heads/*:refs/remotes/frotz/nitfol/* Case 1: Assume both "origin" and "frotz" have remote-tracking branches called "foo", at "refs/remotes/origin/foo" and "refs/remotes/frotz/nitfol/foo" respectively. In this case "git checkout foo" should fail, because there is more than one remote with a "foo" branch. Case 2: Assume only "frotz" have a remote-tracking branch called "foo". In this case "git checkout foo" should succeed, and create a local branch "foo" from "refs/remotes/frotz/nitfol/foo", using remote branch "foo" from "frotz" as its upstream. The current code hardcodes the assumption that all remote-tracking branches must match the "refs/remotes/$remote/*" pattern (which is true for remotes with "conventional" refspecs, but not true for the "frotz" remote above). When running "git checkout foo", the current code looks for exactly one ref matching "refs/remotes/*/foo", hence in the above example, it fails to find "refs/remotes/frotz/nitfol/foo", which causes it to fail both case #1 and #2. The better way to handle the above example is to actually study the fetch refspecs to deduce the candidate remote-tracking branches for "foo"; i.e. assume "foo" is a remote branch being fetched, and then map "refs/heads/foo" through the refspecs in order to get the corresponding remote-tracking branches "refs/remotes/origin/foo" and "refs/remotes/frotz/nitfol/foo". Finally we check which of these happens to exist in the local repo, and if there is exactly one, we have an unambiguous match for "git checkout foo", and may proceed. This fixes most of the failing tests introduced in the previous patch. Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 41c21f2 (branch.c: Validate tracking branches with refspecs instead of refs/remotes/*), we changed the rules for what is considered a valid tracking branch (a.k.a. upstream branch). We now use the configured remotes and their refspecs to determine whether a proposed tracking branch is in fact within the domain of a remote, and we then use that information to deduce the upstream configuration (branch.<name>.remote and branch.<name>.merge). However, with that change, we also check that - in addition to a matching refspec - the result of mapping the tracking branch through that refspec (i.e. the corresponding ref name in the remote repo) happens to start with "refs/heads/". In other words, we require that a tracking branch refers to a _branch_ in the remote repo. Now, consider that you are e.g. setting up an automated building/testing infrastructure for a group of similar "source" repositories. The build/test infrastructure consists of a central scheduler, and a number of build/test "slave" machines that perform the actual build/test work. The scheduler monitors the group of similar repos for changes (e.g. with a periodic "git fetch"), and triggers builds/tests to be run on one or more slaves. Graphically the changes flow between the repos like this: Source git-for-windows#1 -------v ----> Slave git-for-windows#1 / Source git-for-windows#2 -----> Scheduler -----> Slave git-for-windows#2 \ Source git-for-windows#3 -------^ ----> Slave git-for-windows#3 ... ... The scheduler maintains a single Git repo with each of the source repos set up as distinct remotes. The slaves also need access to all the changes from all of the source repos, so they pull from the scheduler repo, but using the following custom refspec: remote.origin.fetch = "+refs/remotes/*:refs/remotes/*" This makes all of the scheduler's remote-tracking branches automatically available as identical remote-tracking branches in each of the slaves. Now, consider what happens if a slave tries to create a local branch with one of the remote-tracking branches as upstream: git branch local_branch --track refs/remotes/source-1/some_branch Git now looks at the configured remotes (in this case there is only "origin", pointing to the scheduler's repo) and sees refs/remotes/source-1/some_branch matching origin's refspec. Mapping through that refspec we find that the corresponding remote ref name is "refs/remotes/source-1/some_branch". However, since this remote ref name does not start with "refs/heads/", we discard it as a suitable upstream, and the whole command fails. This patch adds a testcase demonstrating this failure by creating two source repos ("a" and "b") that are forwarded through a scheduler ("c") to a slave repo ("d"), that then tries create a local branch with an upstream. See the next patch in this series for the exciting conclusion to this story... Reported-by: Per Cederqvist <cederp@opera.com> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In 41c21f2 (branch.c: Validate tracking branches with refspecs instead of refs/remotes/*), we changed the rules for what is considered a valid tracking branch (a.k.a. upstream branch). We now use the configured remotes and their refspecs to determine whether a proposed tracking branch is in fact within the domain of a remote, and we then use that information to deduce the upstream configuration (branch.<name>.remote and branch.<name>.merge). However, with that change, we also check that - in addition to a matching refspec - the result of mapping the tracking branch through that refspec (i.e. the corresponding ref name in the remote repo) happens to start with "refs/heads/". In other words, we require that a tracking branch refers to a _branch_ in the remote repo. Now, consider that you are e.g. setting up an automated building/testing infrastructure for a group of similar "source" repositories. The build/test infrastructure consists of a central scheduler, and a number of build/test "slave" machines that perform the actual build/test work. The scheduler monitors the group of similar repos for changes (e.g. with a periodic "git fetch"), and triggers builds/tests to be run on one or more slaves. Graphically the changes flow between the repos like this: Source git-for-windows#1 -------v ----> Slave git-for-windows#1 / Source git-for-windows#2 -----> Scheduler -----> Slave git-for-windows#2 \ Source git-for-windows#3 -------^ ----> Slave git-for-windows#3 ... ... The scheduler maintains a single Git repo with each of the source repos set up as distinct remotes. The slaves also need access to all the changes from all of the source repos, so they pull from the scheduler repo, but using the following custom refspec: remote.origin.fetch = "+refs/remotes/*:refs/remotes/*" This makes all of the scheduler's remote-tracking branches automatically available as identical remote-tracking branches in each of the slaves. Now, consider what happens if a slave tries to create a local branch with one of the remote-tracking branches as upstream: git branch local_branch --track refs/remotes/source-1/some_branch Git now looks at the configured remotes (in this case there is only "origin", pointing to the scheduler's repo) and sees refs/remotes/source-1/some_branch matching origin's refspec. Mapping through that refspec we find that the corresponding remote ref name is "refs/remotes/source-1/some_branch". However, since this remote ref name does not start with "refs/heads/", we discard it as a suitable upstream, and the whole command fails. This patch adds a testcase demonstrating this failure by creating two source repos ("a" and "b") that are forwarded through a scheduler ("c") to a slave repo ("d"), that then tries create a local branch with an upstream. See the next patch in this series for the exciting conclusion to this story... Reported-by: Per Cederqvist <cederp@opera.com> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have two ways of dealing with empty pathspec: 1. limit it to current prefix 2. match the entire working directory Some commands go with msysgit#1, some msysgit#2. get_pathspec() and parse_pathspec() only support msysgit#1. Make parse_pathspec() reject empty pathspec by default. msysgit#1 and msysgit#2 can be specified via new flags. This makes it more expressive about default behavior at command level. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The main loop in strbuf_utf8_replace() could summed up as:
while ('src' is still valid) {
1) advance 'src' to copy ANSI escape sequences
2) advance 'src' to copy/replace visible characters
}
The problem is after git-for-windows#1, 'src' may have reached the end of the string
(so 'src' points to NUL) and git-for-windows#2 will continue to copy that NUL as if
it's a normal character. Because the output is stored in a strbuf,
this NUL accounted in the 'len' field as well. Check after git-for-windows#1 and
break the loop if necessary.
The test does not look obvious, but the combination of %>>() should
make a call trace like this
show_log()
pretty_print_commit()
format_commit_message()
strbuf_expand()
format_commit_item()
format_and_pad_commit()
strbuf_utf8_replace()
where %C(auto)%d would insert a color reset escape sequence in the end
of the string given to strbuf_utf8_replace() and show_log() uses
fwrite() to send everything to stdout (including the incorrect NUL
inserted by strbuf_utf8_replace)
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Don't chop test_expect_success line into pieces and concatenate with '\'. That's so 2005. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jc/t9001-modernise: t9001: style modernisation phase git-for-windows#5 t9001: style modernisation phase git-for-windows#4 t9001: style modernisation phase git-for-windows#3 t9001: style modernisation phase git-for-windows#2 t9001: style modernisation phase git-for-windows#1
* jc/t9001-modernise: t9001: style modernisation phase git-for-windows#5 t9001: style modernisation phase git-for-windows#4 t9001: style modernisation phase git-for-windows#3 t9001: style modernisation phase git-for-windows#2 t9001: style modernisation phase git-for-windows#1
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The collect_parents() function now is responsible for
1. parsing the commits given on the command line into a list of
commits to be merged;
2. filtering these parents into independent ones; and
3. optionally calling fmt_merge_msg() via prepare_merge_message()
to prepare an auto-generated merge log message, using fake
contents that FETCH_HEAD would have had if these commits were
fetched from the current repository with "git pull . $args..."
Make "git merge FETCH_HEAD" to be the same as the traditional
git merge "$(git fmt-merge-msg <.git/FETCH_HEAD)" $commits
invocation of the command in "git pull", where $commits are the ones
that appear in FETCH_HEAD that are not marked as not-for-merge, by
making it do a bit more, specifically:
- noticing "FETCH_HEAD" is the only "commit" on the command line
and picking the commits that are not marked as not-for-merge as
the list of commits to be merged (substitute for step git-for-windows#1 above);
- letting the resulting list fed to step git-for-windows#2 above;
- doing the step git-for-windows#3 above, using the contents of the FETCH_HEAD
instead of fake contents crafted from the list of commits parsed
in the step git-for-windows#1 above.
Note that this changes the semantics. "git merge FETCH_HEAD" has
always behaved as if the first commit in the FETCH_HEAD file were
directly specified on the command line, creating a two-way merge
whose auto-generated merge log said "merge commit xyz". With this
change, if the previous fetch was to grab multiple branches (e.g.
"git fetch $there topic-a topic-b"), the new world order is to
create an octopus, behaving as if "git pull $there topic-a topic-b"
were run. This is a deliberate change to make that happen, and
can be seen in the changes to t3033 tests.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When ac49f5c (rerere "remaining", 2011-02-16) split out a new helper function check_one_conflict() out of find_conflict() function, so that the latter will use the returned value from the new helper to update the loop control variable that is an index into active_cache[], the new variable incremented the index by one too many when it found a path with only stage msysgit#1 entry at the very end of active_cache[]. This "strange" return value does not have any effect on the loop control of two callers of this function, as they all notice that active_nr+2 is larger than active_nr just like active_nr+1 is, but nevertheless it puzzles the readers when they are trying to figure out what the function is trying to do. In fact, there is no need to do an early return. The code that follows after skipping the stage msysgit#1 entry is fully prepared to handle a case where the entry is at the very end of active_cache[]. Help future readers from unnecessary confusion by dropping an early return. We skip the stage msysgit#1 entry, and if there are stage msysgit#2 and stage msysgit#3 entries for the same path, we diagnose the path as THREE_STAGED (otherwise we say PUNTED), and then we skip all entries for the same path. Signed-off-by: Junio C Hamano <gitster@pobox.com>
A conflicted index can have multiple stage msysgit#1 entries when dealing with a criss-cross merge and using the "resolve" merge strategy. Signed-off-by: Junio C Hamano <gitster@pobox.com>
…entries A conflicted index can have multiple stage msysgit#1 entries when dealing with a criss-cross merge and using the "resolve" merge strategy. Plug the leak by reading only the first one of the same stage entries. Strictly speaking, this fix does change the semantics, in that we used to use the last stage msysgit#1 entry as the common ancestor when doing the plain-vanilla three-way merge, but with the leak fix, we will use the first stage msysgit#1 entry. But it is not a grave backward compatibility breakage. Either way, we are arbitrarily picking one of multiple stage msysgit#1 entries and using it, ignoring others, and there is no meaning in the ordering of these stage msysgit#1 entries. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up and minor fixes. * jc/rerere: (21 commits) rerere: un-nest merge() further rerere: use "struct rerere_id" instead of "char *" for conflict ID rerere: call conflict-ids IDs rerere: further clarify do_rerere_one_path() rerere: further de-dent do_plain_rerere() rerere: refactor "replay" part of do_plain_rerere() rerere: explain the remainder rerere: explain "rerere forget" codepath rerere: explain the primary codepath rerere: explain MERGE_RR management helpers rerere: fix benign off-by-one non-bug and clarify code rerere: explain the rerere I/O abstraction rerere: do not leak mmfile[] for a path with multiple stage msysgit#1 entries rerere: stop looping unnecessarily rerere: drop want_sp parameter from is_cmarker() rerere: report autoupdated paths only after actually updating them rerere: write out each record of MERGE_RR in one go rerere: lift PATH_MAX limitation rerere: plug conflict ID leaks rerere: handle conflicts with multiple stage msysgit#1 entries ...
5 participants
The git completion fix is legit. The logic isn't changed at all, msys just doesn't like the way the original script's loop is structured.
The git submodule fix is a hack. There is a local script variable that was called path, and for some reason, msys wanted to replace it with the contents of the path environment variable. It would be cleaner to fix msys, since that's the root cause, but modifying the script variable name to be something other than path works around the problem.