* What's cooking in git.git (Feb 2011, #05; Wed, 23) @ 2011-02-23 23:26 Junio C Hamano 2011-02-23 23:48 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Jonathan Nieder ` (3 more replies) 0 siblings, 4 replies; 46+ messages in thread From: Junio C Hamano @ 2011-02-23 23:26 UTC (permalink / raw) To: git Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' while commits prefixed with '+' are in 'next'. -------------------------------------------------- [New Topics] * ab/i18n (2011-02-22) 73 commits - i18n: git-shortlog basic messages - i18n: git-revert split up "could not revert/apply" message - i18n: git-revert literal "me" messages - i18n: git-revert "Your local changes" message - i18n: git-revert basic messages - i18n: git-notes GIT_NOTES_REWRITE_MODE error message - i18n: git-notes basic commands - i18n: git-gc "Auto packing the repository" message - i18n: git-gc basic messages - i18n: git-describe basic messages - i18n: git-clean clean.requireForce messages - i18n: git-clean basic messages - i18n: git-bundle basic messages - i18n: git-archive basic messages - i18n: git-status "renamed: " message - i18n: git-status "Initial commit" message - i18n: git-status "Changes to be committed" message - i18n: git-status shortstatus messages - i18n: git-status "nothing to commit" messages - i18n: git-status basic messages - i18n: git-push "prevent you from losing" message - i18n: git-push basic messages - i18n: git-tag tag_template message - i18n: git-tag basic messages - i18n: git-reset "Unstaged changes after reset" message - i18n: git-reset reset_type_names messages - i18n: git-reset basic messages - i18n: git-rm basic messages - i18n: git-mv "bad" messages - i18n: git-mv basic messages - i18n: git-merge "Wonderful" message - i18n: git-merge "You have not concluded your merge" messages - i18n: git-merge "Updating %s..%s" message - i18n: git-merge basic messages - i18n: git-log "--OPT does not make sense" messages - i18n: git-log basic messages - i18n: git-grep "--open-files-in-pager" message - i18n: git-grep basic messages - i18n: git-fetch split up "(non-fast-forward)" message - i18n: git-fetch update_local_ref messages - i18n: git-fetch formatting messages - i18n: git-fetch basic messages - i18n: git-diff basic messages - i18n: git-commit advice messages - i18n: git-commit "enter the commit message" message - i18n: git-commit print_summary messages - i18n: git-commit formatting messages - i18n: git-commit "middle of a merge" message - i18n: git-commit basic messages - i18n: git-checkout "Switched to a .. branch" message - i18n: git-checkout "HEAD is now at" message - i18n: git-checkout describe_detached_head messages - i18n: git-checkout: our/their version message - i18n: git-checkout basic messages - i18n: git-branch "(no branch)" message - i18n: git-branch "git branch -v" messages - i18n: git-branch "Deleted branch [...]" message - i18n: git-branch "remote branch '%s' not found" message - i18n: git-branch basic messages - i18n: git-add "Unstaged changes" message - i18n: git-add "remove '%s'" message - i18n: git-add "did not match any files" message - i18n: git-add "The following paths are ignored" message - i18n: git-add basic messages - i18n: git-clone "Cloning into" message - i18n: git-clone "Cloning into" message - i18n: git-clone basic messages - i18n: git-init "Initialized [...] repository" message - i18n: git-init basic messages - i18n: Makefile: "pot" target to extract messages marked for translation - i18n: do not poison translations unless GIT_GETTEXT_POISON envvar is set - i18n: add GETTEXT_POISON to simulate translated messages unfriendly translator - i18n: add no-op _() and N_() wrappers Re^4-roll, coordinated between Ævar and Jonathan. I'd like to fast-track the basics (especially the bottom 3 patches), and am even tempted to rebase other patches on 'pu' that are not yet in 'next' on top of them, to make the transition easier, so please lend extra sets of eyeballs on an earlier ones to make sure they are sane (I thought they were, but I am far from perfect). * gr/cvsimport-alternative-cvspass-location (2011-02-18) 1 commit - Look for password in both CVS and CVSNT password files. * jc/checkout-orphan-warning (2011-02-18) 1 commit - commit: give final warning when reattaching HEAD to leave commits behind Likes, dislikes? * jh/maint-do-not-track-non-branches (2011-02-17) 1 commit - branch/checkout --track: Ensure that upstream branch is indeed a branch This supersedes "do not track HEAD" from Thomas. * jk/diffstat-binary (2011-02-19) 2 commits (merged to 'next' on 2011-02-23 at 49da967) + diff: don't retrieve binary blobs for diffstat + diff: handle diffstat of rewritten binary files * jk/fail-null-clone (2011-02-17) 1 commit (merged to 'next' on 2011-02-23 at a4217f5) + clone: die when trying to clone missing local path * jk/merge-rename-ux (2011-02-20) 6 commits - pull: propagate --progress to merge - merge: enable progress reporting for rename detection - add inexact rename detection progress infrastructure - commit: stop setting rename limit - bump rename limit defaults (again) - merge: improve inexact rename limit warning The above three all seemed sensible improvements. * jn/test-terminal-punt-on-osx-breakage (2011-02-17) 1 commit (merged to 'next' on 2011-02-23 at d754139) + tests: skip terminal output tests on OS X * js/cherry-pick-usability (2011-02-19) 4 commits (merged to 'next' on 2011-02-23 at 95db30e) + Teach commit about CHERRY_PICK_HEAD + bash: teach __git_ps1 about CHERRY_PICK_HEAD + Introduce CHERRY_PICK_HEAD + t3507: introduce pristine-detach helper * js/detach-doc (2011-02-20) 1 commit (merged to 'next' on 2011-02-21 at c384c3c) + git-checkout.txt: improve detached HEAD documentation * lt/rename-no-extra-copy-detection (2011-02-18) 3 commits (merged to 'next' on 2011-02-23 at 2c1f271) + diffcore-rename: improve estimate_similarity() heuristics + diffcore-rename: properly honor the difference between -M and -C + for_each_hash: allow passing a 'void *data' pointer to callback * mg/rev-list-one-side-only (2011-02-22) 6 commits - t6007: test rev-list --cherry - log --cherry: a synonym - rev-list: --left/right-only are mutually exclusive - rev-list: documentation and test for --left/right-only - t6007: Make sure we test --cherry-pick - revlist.c: introduce --left/right-only for unsymmetric picking * so/submodule-no-update-first-time (2011-02-17) 2 commits (merged to 'next' on 2011-02-23 at 2c6e8c9) + t7406: "git submodule update {--merge|--rebase]" with new submodules + submodule: no [--merge|--rebase] when newly cloned * va/p4 (2011-02-20) 2 commits (merged to 'next' on 2011-02-21 at d981b23) + git-p4: Add copy detection support + git-p4: Improve rename detection support * jc/complete-symmetric-diff (2011-02-23) 1 commit - completion: complete "git diff ...branc<TAB>" * jh/submodule-fetch-on-demand (2011-02-23) 6 commits - submodule update: Don't fetch when the submodule commit is already present - fetch/pull: Don't recurse into a submodule when commits are already present - Submodules: Add 'on-demand' value for the 'fetchRecurseSubmodule' option - config: teach the fetch.recurseSubmodules option the 'on-demand' value - fetch/pull: Add the 'on-demand' value to the --recurse-submodules option - fetch/pull: recurse into submodules when necessary * jk/format-patch-multiline-header (2011-02-23) 2 commits - format-patch: wrap long header lines - strbuf: add fixed-length version of add_wrapped_text * cp/mergetool-beyondcompare (2011-02-18) 1 commit - mergetool--lib: add support for beyond compare May want to have an independent success report on Windows. -------------------------------------------------- [Stalled] * jh/merge-sans-branch (2011-02-10) 4 commits . merge: add support for merging from upstream by default - merge: introduce per-branch-configuration helper function - merge: introduce setup_merge_commit helper function - merge: update the usage information to be more modern There was an objection to the tip one that determines the upstream in a wrong way? * jk/tag-contains (2010-07-05) 4 commits - Why is "git tag --contains" so slow? - default core.clockskew variable to one day - limit "contains" traversals based on commit timestamp - tag: speed up --contains calculation The idea of the bottom one is probably Ok, except that the use of object flags needs to be rethought, or at least the helper needs to be moved to builtin/tag.c to make it clear that it should not be used outside the current usage context. * jc/rename-degrade-cc-to-c (2011-01-06) 3 commits . diffcore-rename: fall back to -C when -C -C busts the rename limit . diffcore-rename: record filepair for rename src . diffcore-rename: refactor "too many candidates" logic * nd/index-doc (2010-09-06) 1 commit . doc: technical details about the index file format -------------------------------------------------- [Cooking] * js/checkout-untracked-symlink (2011-02-20) 2 commits (merged to 'next' on 2011-02-23 at 52a35ce) + do not overwrite untracked symlinks + Demonstrate breakage: checkout overwrites untracked symlink with directory * pw/p4 (2011-02-19) 8 commits (merged to 'next' on 2011-02-21 at 1a7b7d2) + git-p4: support clone --bare + git-p4: decode p4 wildcard characters + git-p4: better message for "git-p4 sync" when not cloned + git-p4: reinterpret confusing p4 message + git-p4: accommodate new move/delete type in p4 + git-p4: add missing newline in initial import message + git-p4: fix key error for p4 problem + git-p4: test script * jh/push-default-upstream-configname (2011-02-16) 1 commit (merged to 'next' on 2011-02-23 at b5c25fa) + push.default: Rename 'tracking' to 'upstream' This is not "renaming" in the sense that breaks existing practice, but giving a new official name and deprecating the existing one. * js/maint-merge-use-prepare-commit-msg-hook (2011-02-14) 1 commit (merged to 'next' on 2011-02-22 at 6458c4b) + merge: honor prepare-commit-msg hook * mg/patch-id (2011-02-17) 2 commits (merged to 'next' on 2011-02-22 at 6f4acd8) + git-patch-id: do not trip over "no newline" markers + git-patch-id: test for "no newline" markers * mg/placeholders-are-lowercase (2011-02-17) 5 commits (merged to 'next' on 2011-02-22 at 2754e21) + Make <identifier> lowercase in Documentation + Make <identifier> lowercase as per CodingGuidelines + Make <identifier> lowercase as per CodingGuidelines + Make <identifier> lowercase as per CodingGuidelines + CodingGuidelines: downcase placeholders in usage messages * mo/perl-bidi-pipe-envfix (2011-02-15) 1 commit (merged to 'next' on 2011-02-15 at c36e816) + perl: command_bidi_pipe() method should set-up git environmens Looked reasonable. * nd/sorted-builtin-command-list (2011-02-15) 1 commit (merged to 'next' on 2011-02-22 at 91fccd1) + git.c: reorder builtin command list * sp/maint-smart-http-sans-100-continue (2011-02-15) 1 commit (merged to 'next' on 2011-02-15 at 553e3e5) + smart-http: Don't use Expect: 100-Continue * jc/grep--no-index-pathspec-fix (2011-02-16) 1 commit (merged to 'next' on 2011-02-23 at 58b03b1) + grep --no-index: honor pathspecs correctly (this branch uses nd/struct-pathspec; is tangled with en/object-list-with-pathspec.) * mz/rebase (2011-02-09) 32 commits (merged to 'next' on 2011-02-22 at 3219155) + rebase: use @{upstream} if no upstream specified + rebase -i: remove unnecessary state rebase-root + rebase -i: don't read unused variable preserve_merges + git-rebase--am: remove unnecessary --3way option + rebase -m: don't print exit code 2 when merge fails + rebase -m: remember allow_rerere_autoupdate option + rebase: remember strategy and strategy options + rebase: remember verbose option + rebase: extract code for writing basic state + rebase: factor out sub command handling + rebase: make -v a tiny bit more verbose + rebase -i: align variable names + rebase: show consistent conflict resolution hint + rebase: extract am code to new source file + rebase: extract merge code to new source file + rebase: remove $branch as synonym for $orig_head + rebase -i: support --stat + rebase: factor out call to pre-rebase hook + rebase: factor out clean work tree check + rebase: factor out reference parsing + rebase: reorder validation steps + rebase -i: remove now unnecessary directory checks + rebase: factor out command line option processing + rebase: align variable content + rebase: align variable names + rebase: stricter check of standalone sub command + rebase: act on command line outside parsing loop + rebase: improve detection of rebase in progress + rebase: remove unused rebase state 'prev_head' + rebase: read state outside loop + rebase: refactor reading of state + rebase: clearer names for directory variables Minor UI regression was reported but otherwise it looked like that the topic is in a good shape. * lp/config-vername-check (2011-02-01) 2 commits (merged to 'next' on 2011-02-23 at 426d48d) + Disallow empty section and variable names + Sanity-check config variable names * mz/rerere-remaining (2011-02-16) 2 commits (merged to 'next' on 2011-02-22 at fa2d5ab) + mergetool: don't skip modify/remove conflicts + rerere "remaining" Looked much better than my weatherbaloon patch. * nd/hash-object-sanity (2011-02-05) 1 commit (merged to 'next' on 2011-02-22 at 09acf6f) + Make hash-object more robust against malformed objects * hv/mingw-fs-funnies (2011-02-07) 5 commits (merged to 'next' on 2011-02-09 at 3d0bb1a) + mingw_rmdir: set errno=ENOTEMPTY when appropriate + mingw: add fallback for rmdir in case directory is in use + mingw: make failures to unlink or move raise a question + mingw: work around irregular failures of unlink on windows + mingw: move unlink wrapper to mingw.c Rerolled and seems ready to move forward. * nd/struct-pathspec (2011-01-31) 22 commits (merged to 'next' on 2011-02-09 at b1e64ee) + t6004: add pathspec globbing test for log family + t7810: overlapping pathspecs and depth limit + grep: drop pathspec_matches() in favor of tree_entry_interesting() + grep: use writable strbuf from caller for grep_tree() + grep: use match_pathspec_depth() for cache/worktree grepping + grep: convert to use struct pathspec + Convert ce_path_match() to use match_pathspec_depth() + Convert ce_path_match() to use struct pathspec + struct rev_info: convert prune_data to struct pathspec + pathspec: add match_pathspec_depth() + tree_entry_interesting(): optimize wildcard matching when base is matched + tree_entry_interesting(): support wildcard matching + tree_entry_interesting(): fix depth limit with overlapping pathspecs + tree_entry_interesting(): support depth limit + tree_entry_interesting(): refactor into separate smaller functions + diff-tree: convert base+baselen to writable strbuf + glossary: define pathspec + Move tree_entry_interesting() to tree-walk.c and export it + tree_entry_interesting(): remove dependency on struct diff_options + Convert struct diff_options to use struct pathspec + diff-no-index: use diff_tree_setup_paths() + Add struct pathspec (this branch is used by en/object-list-with-pathspec and jc/grep--no-index-pathspec-fix.) * en/object-list-with-pathspec (2010-09-20) 2 commits (merged to 'next' on 2011-02-09 at ccf6c6a) + Add testcases showing how pathspecs are handled with rev-list --objects + Make rev-list --objects work together with pathspecs (this branch uses nd/struct-pathspec; is tangled with jc/grep--no-index-pathspec-fix.) * uk/checkout-ambiguous-ref (2011-02-15) 5 commits (merged to 'next' on 2011-02-15 at 645dad6) + Rename t2019 with typo "amiguous" that meant "ambiguous" + checkout: rearrange update_refs_for_switch for clarity + checkout: introduce --detach synonym for "git checkout foo^{commit}" + checkout: split off a function to peel away branchname arg (merged to 'next' on 2011-02-03 at 9044724) + checkout: fix bug with ambiguous refs The topic has become about "checkout --detach" ;-). ^ permalink raw reply [flat|nested] 46+ messages in thread
* ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-23 23:26 What's cooking in git.git (Feb 2011, #05; Wed, 23) Junio C Hamano @ 2011-02-23 23:48 ` Jonathan Nieder 2011-02-24 1:16 ` Junio C Hamano 2011-02-24 9:56 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Erik Faye-Lund 2011-02-23 23:52 ` What's cooking in git.git (Feb 2011, #05; Wed, 23) Johan Herland ` (2 subsequent siblings) 3 siblings, 2 replies; 46+ messages in thread From: Jonathan Nieder @ 2011-02-23 23:48 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason Junio C Hamano wrote: > * ab/i18n (2011-02-22) 73 commits [...] > - i18n: do not poison translations unless GIT_GETTEXT_POISON envvar is set > - i18n: add GETTEXT_POISON to simulate translated messages unfriendly translator > - i18n: add no-op _() and N_() wrappers > > Re^4-roll, coordinated between Ævar and Jonathan. > > I'd like to fast-track the basics (especially the bottom 3 patches), and > am even tempted to rebase other patches on 'pu' that are not yet in 'next' > on top of them, to make the transition easier, so please lend extra sets > of eyeballs on an earlier ones to make sure they are sane (I thought they > were, but I am far from perfect). The commit message for the second one seems to have been mangled: i18n: add GETTEXT_POISON to simulate translated messages unfriendly translator Before, it said "simulate unfriendly translator", as in "turn on this option to see what the translator from hell could do to your program". (Well, it's not _that_ bad because it takes out format strings.) I still don't like the #-sign business in this commit. Couldn't it be split into a separate patch, not to be applied until just before the strings in commit/tag/wt-status are marked for translation? There is also a patch out there to make this use rot13, which I am somewhat fond of. Unfortunately, it leaks (because it is not clear how long translated strings are supposed to last). I'd be happy with renaming use_poison() to gettext_poison() or similar. I suppose that is not urgent. Regards, Jonathan ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-23 23:48 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Jonathan Nieder @ 2011-02-24 1:16 ` Junio C Hamano 2011-02-24 2:55 ` Ævar Arnfjörð Bjarmason 2011-02-24 9:56 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Erik Faye-Lund 1 sibling, 1 reply; 46+ messages in thread From: Junio C Hamano @ 2011-02-24 1:16 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, Ævar Arnfjörð Bjarmason Jonathan Nieder <jrnieder@gmail.com> writes: > The commit message for the second one seems to have been mangled: > > i18n: add GETTEXT_POISON to simulate translated messages unfriendly translator Yeah, I think I was fooled by the header folding while fixing things up inside the mailbox. > I still don't like the #-sign business in this commit. Couldn't it > be split into a separate patch, not to be applied until just before > the strings in commit/tag/wt-status are marked for translation? That might be a sensible thing to do. Ævar what do you think? > There is also a patch out there to make this use rot13, which I am > somewhat fond of. Unfortunately, it leaks (because it is not clear > how long translated strings are supposed to last). Yeah I would imagine it would leak. Also blindly running rot13 to turn %d into %q is probably not what you want. > I'd be happy with renaming use_poison() to gettext_poison() or > similar. I suppose that is not urgent. I would prefer to be able to merge the first handful to 'master' sooner rather than later to minimize the damage to in-flight topics, though. Thanks. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 1:16 ` Junio C Hamano @ 2011-02-24 2:55 ` Ævar Arnfjörð Bjarmason 2011-02-24 3:14 ` Jonathan Nieder 2011-02-24 4:00 ` ab/i18n Jonathan Nieder 0 siblings, 2 replies; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 2:55 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, git On Thu, Feb 24, 2011 at 02:16, Junio C Hamano <gitster@pobox.com> wrote: > Jonathan Nieder <jrnieder@gmail.com> writes: > >> The commit message for the second one seems to have been mangled: >> >> i18n: add GETTEXT_POISON to simulate translated messages unfriendly translator > > Yeah, I think I was fooled by the header folding while fixing things up > inside the mailbox. > >> I still don't like the #-sign business in this commit. Couldn't it >> be split into a separate patch, not to be applied until just before >> the strings in commit/tag/wt-status are marked for translation? > > That might be a sensible thing to do. Ævar what do you think? It could, but I don't want to spend time on it. I think it's fine that the choice of string is documented in the commit that introduces it. >> There is also a patch out there to make this use rot13, which I am >> somewhat fond of. Unfortunately, it leaks (because it is not clear >> how long translated strings are supposed to last). > > Yeah I would imagine it would leak. Also blindly running rot13 to turn %d > into %q is probably not what you want. Gettext gives you a pointer to the string inside a mmaped .mo file, effectively, so the caller doesn't have to care about it. So you can't write something that doesn't leak memory unless you preserve that characteristic. But it's a debugging mode, it doesn't matter IMO if it leaks memory. Anyway, to be blunt I really don't see the point of fiddling around with this bit so much. Whether it's a `"GETTEXT POISON"` constant or `rot13(msgid)` the same tests will fail. So it's not functionally changing what the feature was for in the first place, just adding more complexity. Of course there might be cases where a test will fail because it's supposed to end in \n but the poison string doesn't, but since none of them did I didn't worry about that. And even if one did I'd probably just mark it as C_LOCALE_ONLY anyway. Since the intent is to make the person doing the i18n work not break plumbing, not to perfectly annotate our test suite. I'd much rather have a few more tests skipped under this poison mode than 20 extra lines of C code that effectively give us nothing to maintain. If someone else wants to do the work I don't care. I just think there's more important things to worry about. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 2:55 ` Ævar Arnfjörð Bjarmason @ 2011-02-24 3:14 ` Jonathan Nieder 2011-02-24 10:38 ` Ævar Arnfjörð Bjarmason 2011-02-24 4:00 ` ab/i18n Jonathan Nieder 1 sibling, 1 reply; 46+ messages in thread From: Jonathan Nieder @ 2011-02-24 3:14 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Ævar Arnfjörð Bjarmason wrote: > Anyway, to be blunt I really don't see the point of fiddling around > with this bit so much. Whether it's a `"GETTEXT POISON"` constant or > `rot13(msgid)` the same tests will fail. Because with rot13 you can decode the message and find the problematic code? > If someone else wants to do the work I don't care. I just think > there's more important things to worry about. I already wrote a patch. I imagine you don't intend it this way, but I keep on hearing "we shouldn't do what you've already done. It's too much trouble. You wasted your time." ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 3:14 ` Jonathan Nieder @ 2011-02-24 10:38 ` Ævar Arnfjörð Bjarmason 2011-02-24 10:45 ` Jonathan Nieder 2011-02-24 11:00 ` Jonathan Nieder 0 siblings, 2 replies; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 10:38 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, git On Thu, Feb 24, 2011 at 04:14, Jonathan Nieder <jrnieder@gmail.com> wrote: > Ævar Arnfjörð Bjarmason wrote: > >> Anyway, to be blunt I really don't see the point of fiddling around >> with this bit so much. Whether it's a `"GETTEXT POISON"` constant or >> `rot13(msgid)` the same tests will fail. > > Because with rot13 you can decode the message and find the problematic > code? > >> If someone else wants to do the work I don't care. I just think >> there's more important things to worry about. > > I already wrote a patch. I imagine you don't intend it this way, but > I keep on hearing "we shouldn't do what you've already done. It's too > much trouble. You wasted your time." I don't mean it that way at all, I just mean that as a comment to *this* particular patch series I don't think it's something we have to worry about. I.e. if you look at this in context we have a 170 gettext patch series to trickle in gradually. Right now we have 70 pending with only the C translations. Hopefully we can fast-track this to have around 150 of those in in the next month or so. What I want to focus on is getting this right for users of Git, and I think it's already good enough for that purpose. I'm not worried about an embedded debug testing mode that to my knowledge only you and I have used so far. If it has some changes for improvement I think they can be dealt with later, doing it at this point means more code to review. Right now the C code in the series is around 10 lines, doing something like the rot13 implementation would at least triple that, making in not leak memory (and hence have automated tools complain) would be even more code. Again, don't get me wrong. I really appreciate your work. I just don't think this is something we need to worry about now. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 10:38 ` Ævar Arnfjörð Bjarmason @ 2011-02-24 10:45 ` Jonathan Nieder 2011-02-24 11:00 ` Jonathan Nieder 1 sibling, 0 replies; 46+ messages in thread From: Jonathan Nieder @ 2011-02-24 10:45 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Ævar Arnfjörð Bjarmason wrote: > I don't mean it that way at all, I just mean that as a comment to > *this* particular patch series I don't think it's something we have to > worry about. Then we're in violent agreement, I suppose. I don't think anyone meant to imply that rot13 or anything of the kind should be snuck in here. Rereading my message, I see how it can be read that way --- sorry about that. I had only meant to give a heads up about something coming in the future. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 10:38 ` Ævar Arnfjörð Bjarmason 2011-02-24 10:45 ` Jonathan Nieder @ 2011-02-24 11:00 ` Jonathan Nieder 2011-02-25 21:48 ` Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 46+ messages in thread From: Jonathan Nieder @ 2011-02-24 11:00 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Ævar Arnfjörð Bjarmason wrote: > I don't mean it that way at all, I just mean that as a comment to > *this* particular patch series I don't think it's something we have to > worry about. This is really tiring and unpleasant. I don't want to stand in the way of a translated git happening or to take on the project myself so I can't just decree "it will be like so". It's your and Junio's (and lots other people's, of course) code. But that means that for me to be able to help, I need to be able to say, "here's a suggested change" and get an "okay" or "no, here's what's wrong with that and how you can improve it". And that just doesn't seem to be happening now. I don't know how to fix it, but I thought I should explain why I am probably so frustrating to work with right now. Sorry about that. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 11:00 ` Jonathan Nieder @ 2011-02-25 21:48 ` Ævar Arnfjörð Bjarmason 2011-02-26 5:14 ` Jonathan Nieder 0 siblings, 1 reply; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-25 21:48 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, git On Thu, Feb 24, 2011 at 12:00, Jonathan Nieder <jrnieder@gmail.com> wrote: > Ævar Arnfjörð Bjarmason wrote: > >> I don't mean it that way at all, I just mean that as a comment to >> *this* particular patch series I don't think it's something we have to >> worry about. > > This is really tiring and unpleasant. I don't want to stand in the > way of a translated git happening or to take on the project myself > so I can't just decree "it will be like so". > > It's your and Junio's (and lots other people's, of course) code. But > that means that for me to be able to help, I need to be able to say, > "here's a suggested change" and get an "okay" or "no, here's what's > wrong with that and how you can improve it". > > And that just doesn't seem to be happening now. I don't know how to > fix it, but I thought I should explain why I am probably so > frustrating to work with right now. First off, I really appreciate having your input and help on all this, really. And I didn't mean to cause frustration. As for whether your code looks good, I've found it to be better than the stuff I've come up with. Sorry about not being more clear about that, and giving "ok"'s etc. I think the improvements you've made are great, and if you're willing to re-submit a better version of the series that would be even better (depending on the status of the current one wrt Junio etc). My apprehensiveness about adding extra things and fixing up the series has really been about one thing, and one thing only. Which is that I don't often have time to spend on git.git anymore, and haven't submitted the series since October of last year. I was hoping that I could get it into a form that would be acceptable for fast-tracking. I.e. I could make a tiny patch at the tip of the series, prove that it was OK, then the other 50 C/Shell etc. patches could get in quickly. Anyway, with the timeline Junio has in mind *that* probably won't be possible. I'm rapidly approaching a time where I again won't have time for Git. But just getting this in would be great, I can submit more stuff later when I have time, and it'll be less to juggle at once since there'll already be something in git.git:master. And if someone else wants to pick it up and shepard it along that would be fantastic as well. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-25 21:48 ` Ævar Arnfjörð Bjarmason @ 2011-02-26 5:14 ` Jonathan Nieder 0 siblings, 0 replies; 46+ messages in thread From: Jonathan Nieder @ 2011-02-26 5:14 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Ævar Arnfjörð Bjarmason wrote: > First off, I really appreciate having your input and help on all this, > really. And I didn't mean to cause frustration. The series seems to be un-stuck now, so frustration gone. Sorry, I'm no good at this sort of thing. Thanks for translated git, gettext poison, "prove"-able tests and other assorted neat toys. :) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n 2011-02-24 2:55 ` Ævar Arnfjörð Bjarmason 2011-02-24 3:14 ` Jonathan Nieder @ 2011-02-24 4:00 ` Jonathan Nieder 1 sibling, 0 replies; 46+ messages in thread From: Jonathan Nieder @ 2011-02-24 4:00 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git Ævar Arnfjörð Bjarmason wrote: > On Thu, Feb 24, 2011 at 02:16, Junio C Hamano <gitster@pobox.com> wrote: >> Jonathan Nieder <jrnieder@gmail.com> writes: >>> I still don't like the #-sign business in this commit. Couldn't it >>> be split into a separate patch, not to be applied until just before >>> the strings in commit/tag/wt-status are marked for translation? >> >> That might be a sensible thing to do. Ævar what do you think? > > It could, but I don't want to spend time on it. That doesn't answer the question, does it? The question is whether you'd be okay with the change (in which case we should do it --- it's not like there's a shortage of people willing to write a one-line patch) or not (in which case we shouldn't). >> Yeah I would imagine it would leak. Also blindly running rot13 to turn %d >> into %q is probably not what you want. The rot13 patch turns %d into 5q (and 5q into "d). I think that aspect can wait for later, of course --- a fixed string is not actively bad or confusing. > Of course there might be cases where a test will fail because it's > supposed to end in \n but the poison string doesn't, but since none of > them did I didn't worry about that. Right, that's a downside to the rot13 patch --- it takes \n to \n so it wouldn't catch such cases. Easily fixable, though. > I'd much > rather have a few more tests skipped under this poison mode than 20 > extra lines of C code that effectively give us nothing No disagreement here. I think some cases (e.g., adding '# ' before each line of the translated status hints) would give us something, namely protection against hard-to-debug translation bugs. Sorry for the grouchiness, and hope that helps. Jonathan ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-23 23:48 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Jonathan Nieder 2011-02-24 1:16 ` Junio C Hamano @ 2011-02-24 9:56 ` Erik Faye-Lund 2011-02-24 10:27 ` ab/i18n Jonathan Nieder 2011-02-24 10:40 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Ævar Arnfjörð Bjarmason 1 sibling, 2 replies; 46+ messages in thread From: Erik Faye-Lund @ 2011-02-24 9:56 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Junio C Hamano, git, Ævar Arnfjörð On Thu, Feb 24, 2011 at 12:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: > There is also a patch out there to make this use rot13, which I am > somewhat fond of. Unfortunately, it leaks (because it is not clear > how long translated strings are supposed to last). > I like the idea, but perhaps we could auto-generate a Pig Latin translation or something instead? Pig Latin has the benefit over rot-13 of the strings being of a different length than the original, which might trigger some bugs that same-length translations might not... ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n 2011-02-24 9:56 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Erik Faye-Lund @ 2011-02-24 10:27 ` Jonathan Nieder 2011-02-24 19:11 ` ab/i18n Junio C Hamano 2011-02-24 10:40 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Ævar Arnfjörð Bjarmason 1 sibling, 1 reply; 46+ messages in thread From: Jonathan Nieder @ 2011-02-24 10:27 UTC (permalink / raw) To: Erik Faye-Lund; +Cc: Junio C Hamano, git, Ævar Arnfjörð Erik Faye-Lund wrote: > I like the idea, but perhaps we could auto-generate a Pig Latin > translation or something instead? Sorry for the fuss. If I understand correctly, the remaining issues with the current ab/i18n branch in 'pu' are: * The series is too long to read in one sitting. Suggested fix: deal with some arbitrary early subset (35 patches? I'd even prefer around 20) first. * Patch 2 (add GETTEXT_POISON) is conceptually complicated since its arbitrary string is not arbitrary. Suggested fix: split it into two patches. But I am still not sure if that's considered acceptable? * Patch 2 squats on a valuable use_poison() identifier space. Suggested fix: rename it to gettext_poison(). * Patches 5 (i18n: git-init basic messages) and onward do not explain "we are marking strings for translation, in preparation for translating them later" in their commit messages. Suggested fix: use titles like «i18n: mark some "git init" messages for translation». Or ignore the problem --- it's not a big deal. * We haven't run an automated tool to check that this is a no-op in the -UGETTEXT_POISON case. Suggested fix: build without debugging symbols and compare the binaries. Or invent a tool to check patches. Or just use our eyes, like we always have. Does that sound like a fair summary? I'd be happy to reroll the first 30 or so patches following whatever approach is the consensus for these things to move this forward. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n 2011-02-24 10:27 ` ab/i18n Jonathan Nieder @ 2011-02-24 19:11 ` Junio C Hamano 0 siblings, 0 replies; 46+ messages in thread From: Junio C Hamano @ 2011-02-24 19:11 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Erik Faye-Lund, git, Ævar Arnfjörð Jonathan Nieder <jrnieder@gmail.com> writes: > * The series is too long to read in one sitting. > > Suggested fix: deal with some arbitrary early subset (35 patches? > I'd even prefer around 20) first. Let's concentrate on making sure that the first four (i18n foundation, no markings) gets merged to 'master' first, using the early ones (say, "init", "clone", "add" and "branch") as a sanity checks. And then we can stagger the remainder, perhaps at most 10 in one batch but preferrably smaller, two to three batches a week merged to 'next' and then down to 'master', over a few weeks. > * Patch 2 (add GETTEXT_POISON) is conceptually complicated since > its arbitrary string is not arbitrary. > > Suggested fix: split it into two patches. But I am still not sure > if that's considered acceptable? See bottom. > * Patch 2 squats on a valuable use_poison() identifier space. > > Suggested fix: rename it to gettext_poison(). For this, I can just "rebase -i"; I don't think there is anything controversial nor tricky. > * Patches 5 (i18n: git-init basic messages) and onward do not > explain "we are marking strings for translation, in > preparation for translating them later" in their commit messages. > > Suggested fix: use titles like «i18n: mark some "git init" messages > for translation». Or ignore the problem --- it's not a big deal. If you want to see a mixed patch that has both logic reorganization and message marking in one change, e.g. i18n: git-revert split up "could not revert/apply" message, split into one that only reorganizes the code structure and the other that only marks literal string, your "mark for translation" would start making sense. The other half won't even have "i18n:" prefix in that case. If we go that route, it would be better to have the code restructuring patches merged to 'master' before (and independent from) i18n. But the ones I took a look at didn't seem too bad (e.g. i18n: git-revert "Your local changes" message). I think Ævar did a good job splitting the changes into a reasonably small bite-sized chunks in this round. Assuming that we are not going to do that "even finer" split, I think the patches we have on 'pu' are descriptive enough already. They look like this: i18n: git-tag tag_template message i18n: git-mv "bad" messages > Does that sound like a fair summary? To me it does. Thanks. > I'd be happy to reroll the first 30 or so patches following whatever > approach is the consensus for these things to move this forward. I tend to think that except for the GETTEXT_POISON bit what we have is mostly fine. Can you give two weather-balloon counterproposal patches for "add GETTEXT_POISON" and "i18n: git-status "Changes to be committed" message" to illustrate what you have in mind, and perhaps list the patches in Ævar's series that need similar changes as you would need for the latter, to illustrate the extent of damage a careless translator can cause by omitting '#' from the beginning? Would that help the topic get unstuck? ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) 2011-02-24 9:56 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Erik Faye-Lund 2011-02-24 10:27 ` ab/i18n Jonathan Nieder @ 2011-02-24 10:40 ` Ævar Arnfjörð Bjarmason 1 sibling, 0 replies; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 10:40 UTC (permalink / raw) To: kusmabite; +Cc: Jonathan Nieder, Junio C Hamano, git On Thu, Feb 24, 2011 at 10:56, Erik Faye-Lund <kusmabite@gmail.com> wrote: > On Thu, Feb 24, 2011 at 12:48 AM, Jonathan Nieder <jrnieder@gmail.com> wrote: >> There is also a patch out there to make this use rot13, which I am >> somewhat fond of. Unfortunately, it leaks (because it is not clear >> how long translated strings are supposed to last). >> > > I like the idea, but perhaps we could auto-generate a Pig Latin > translation or something instead? Pig Latin has the benefit over > rot-13 of the strings being of a different length than the original, > which might trigger some bugs that same-length translations might > not... At the risk of sounding too succinct. "dudes" :) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-23 23:26 What's cooking in git.git (Feb 2011, #05; Wed, 23) Junio C Hamano 2011-02-23 23:48 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Jonathan Nieder @ 2011-02-23 23:52 ` Johan Herland 2011-02-24 0:43 ` Junio C Hamano 2011-02-24 0:32 ` Ævar Arnfjörð Bjarmason 2011-02-26 10:24 ` Adding Beyond Compare as a merge tool, was: " Sebastian Schuberth 3 siblings, 1 reply; 46+ messages in thread From: Johan Herland @ 2011-02-23 23:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thursday 24 February 2011, Junio C Hamano wrote: > Here are the topics that have been cooking. Commits prefixed with '-' > are only in 'pu' while commits prefixed with '+' are in 'next'. > > [...] > * jh/maint-do-not-track-non-branches (2011-02-17) 1 commit > - branch/checkout --track: Ensure that upstream branch is indeed a > branch > > This supersedes "do not track HEAD" from Thomas. What do you mean by "supersedes"? It builds on top of Thomas' patch (v2 was rebased on top it), but it does not _replace_ Thomas' patch. ...Johan -- Johan Herland, <johan@herland.net> www.herland.net ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-23 23:52 ` What's cooking in git.git (Feb 2011, #05; Wed, 23) Johan Herland @ 2011-02-24 0:43 ` Junio C Hamano 2011-02-24 1:04 ` Johan Herland 0 siblings, 1 reply; 46+ messages in thread From: Junio C Hamano @ 2011-02-24 0:43 UTC (permalink / raw) To: Johan Herland; +Cc: git Johan Herland <johan@herland.net> writes: > On Thursday 24 February 2011, Junio C Hamano wrote: >> Here are the topics that have been cooking. Commits prefixed with '-' >> are only in 'pu' while commits prefixed with '+' are in 'next'. >> >> [...] >> * jh/maint-do-not-track-non-branches (2011-02-17) 1 commit >> - branch/checkout --track: Ensure that upstream branch is indeed a >> branch >> >> This supersedes "do not track HEAD" from Thomas. > > What do you mean by "supersedes"? It builds on top of Thomas' patch (v2 was > rebased on top it), but it does not _replace_ Thomas' patch. I judge that the earlier one to special case only "HEAD" a failed attempt to the wider problem; what was posted to the list might have been relative to it, but what is queued reverts the effect of the previous patch and is relative to the mainline. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 0:43 ` Junio C Hamano @ 2011-02-24 1:04 ` Johan Herland 0 siblings, 0 replies; 46+ messages in thread From: Johan Herland @ 2011-02-24 1:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thursday 24 February 2011, Junio C Hamano wrote: > Johan Herland <johan@herland.net> writes: > > On Thursday 24 February 2011, Junio C Hamano wrote: > >> Here are the topics that have been cooking. Commits prefixed with '-' > >> are only in 'pu' while commits prefixed with '+' are in 'next'. > >> > >> [...] > >> * jh/maint-do-not-track-non-branches (2011-02-17) 1 commit > >> > >> - branch/checkout --track: Ensure that upstream branch is indeed a > >> > >> branch > >> > >> This supersedes "do not track HEAD" from Thomas. > > > > What do you mean by "supersedes"? It builds on top of Thomas' patch (v2 > > was rebased on top it), but it does not _replace_ Thomas' patch. > > I judge that the earlier one to special case only "HEAD" a failed attempt > to the wider problem; what was posted to the list might have been > relative to it, but what is queued reverts the effect of the previous > patch and is relative to the mainline. Ah, I see. I'm good with that. :) ...Johan -- Johan Herland, <johan@herland.net> www.herland.net ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-23 23:26 What's cooking in git.git (Feb 2011, #05; Wed, 23) Junio C Hamano 2011-02-23 23:48 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Jonathan Nieder 2011-02-23 23:52 ` What's cooking in git.git (Feb 2011, #05; Wed, 23) Johan Herland @ 2011-02-24 0:32 ` Ævar Arnfjörð Bjarmason 2011-02-24 0:57 ` Junio C Hamano 2011-02-24 9:45 ` Erik Faye-Lund 2011-02-26 10:24 ` Adding Beyond Compare as a merge tool, was: " Sebastian Schuberth 3 siblings, 2 replies; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 0:32 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Feb 24, 2011 at 00:26, Junio C Hamano <gitster@pobox.com> wrote: > * ab/i18n (2011-02-22) 73 commits > [...] > > Re^4-roll, coordinated between Ævar and Jonathan. > > I'd like to fast-track the basics (especially the bottom 3 patches), and > am even tempted to rebase other patches on 'pu' that are not yet in 'next' > on top of them, to make the transition easier, so please lend extra sets > of eyeballs on an earlier ones to make sure they are sane (I thought they > were, but I am far from perfect). Great that we're moving this forward. After this has made it to next (or master) I'm going to fix this up a bit and submit the shellscript i18n-ize patches: https://github.com/avar/git/compare/5bd8b10...8fd2407 Open issues: * Write documentation for git-sh-i18n.sh and git-sh-i18n--envsubst like we have for git-sh-setup (already in WIP form). * git-sh-i18n--envsubst is still too fat: $ ldd -r git-sh-i18n--envsubst linux-vdso.so.1 => (0x00007fffc60fd000) libz.so.1 => /usr/lib/libz.so.1 (0x00007f25cff9e000) libcrypto.so.0.9.8 => /usr/lib/libcrypto.so.0.9.8 (0x00007f25cfbfd000) libpthread.so.0 => /lib/libpthread.so.0 (0x00007f25cf9e0000) libc.so.6 => /lib/libc.so.6 (0x00007f25cf67f000) libdl.so.2 => /lib/libdl.so.2 (0x00007f25cf47b000) /lib64/ld-linux-x86-64.so.2 (0x00007f25d01c0000) It only needs to link to libc, but I didn't find out when I last checked how to convince the Makefile to only link against that. Help welcome :) * Deal with the changes in 92c62a3f4f93432c0c82e3031a9e64e03ba290f7: $ git --no-pager grep -A1 abomination *.sh git-pull.sh: # XXX: This is an abomination git-pull.sh- require_clean_work_tree "pull with rebase" "Please commit or stash them." The changes Ramkumar Ramachandra made in 92c62a3f4f, while good, are hard to square with i18n. I think I'll just leave those bits untranslated for now and deal with them later, since I'm trying to keep this minimal. And then there's the issue that unlike the C patches these will not be a no-op that'll be optimized away by the compiler. We'll be calling an external program for displaying messages. While this is a trivial cost on Unix (especially in the context we're using it, i.e. not in tight loops) it's more expensive on Windows. I don't see any way to deal with that short of implementing some pre-processor, but I think the cost is worth it, but others might disagree of course. Anyway, I can submit these patches (around 53) real soon, or wait until the current series settles. It's the same to me, which would you prefer? > * gr/cvsimport-alternative-cvspass-location (2011-02-18) 1 commit > - Look for password in both CVS and CVSNT password files. Given that this solves someone's problem I don't think there's any harm in letting it through. The code and overall behavior is somewhat nasty, but then so is the rest of git-cvs*.perl, so meh. > * jc/checkout-orphan-warning (2011-02-18) 1 commit > - commit: give final warning when reattaching HEAD to leave commits behind I like this sort of thing. Generally speaking I don't mind Git being more friendly and verbose about this sort of thing. I.e. for common pitfalls print a few lines of help because it'll help more than it hurts. I think we've been to conservative about that in the past. There's a grammar error here though if I'm not mistaken: w git ((f11f53c...) $%) $ ./git co pu Warning: you are leaving 1 commit behind that are not connected to any of your branches: For the singular this should be "1 commit behind which is not corrected to any of your branches". We're also being somewhat inaccurate by omission here, since we won't give this warning if the commit is reachable from any named referenc, i.e. if the user just tagged it. So perhaps this should say something about "not reachable from any named reference" or something, but then again that would confuse the sort of users that need this the most. > * jh/maint-do-not-track-non-branches (2011-02-17) 1 commit > - branch/checkout --track: Ensure that upstream branch is indeed a branch I like it. > * jk/merge-rename-ux (2011-02-20) 6 commits > - pull: propagate --progress to merge > - merge: enable progress reporting for rename detection > - add inexact rename detection progress infrastructure > - commit: stop setting rename limit > - bump rename limit defaults (again) > - merge: improve inexact rename limit warning > > The above three all seemed sensible improvements. Agreed. > * jn/test-terminal-punt-on-osx-breakage (2011-02-17) 1 commit > (merged to 'next' on 2011-02-23 at d754139) > + tests: skip terminal output tests on OS X Looks good, and maybe I'll debug the Perl one of these days. > * jk/tag-contains (2010-07-05) 4 commits > - Why is "git tag --contains" so slow? > - default core.clockskew variable to one day > - limit "contains" traversals based on commit timestamp > - tag: speed up --contains calculation > > The idea of the bottom one is probably Ok, except that the use of object > flags needs to be rethought, or at least the helper needs to be moved to > builtin/tag.c to make it clear that it should not be used outside the > current usage context. I really like this as noted elsewhere. But it seems that it would go down better if the helper was submitted later on. > * jh/push-default-upstream-configname (2011-02-16) 1 commit > (merged to 'next' on 2011-02-23 at b5c25fa) > + push.default: Rename 'tracking' to 'upstream' > > This is not "renaming" in the sense that breaks existing practice, but > giving a new official name and deprecating the existing one. I like the new name. > * mo/perl-bidi-pipe-envfix (2011-02-15) 1 commit > (merged to 'next' on 2011-02-15 at c36e816) > + perl: command_bidi_pipe() method should set-up git environmens > > Looked reasonable. Yeah, but fix up the typo in the subject. Should be "environments" :) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 0:32 ` Ævar Arnfjörð Bjarmason @ 2011-02-24 0:57 ` Junio C Hamano 2011-02-24 1:37 ` Ævar Arnfjörð Bjarmason 2011-02-24 9:45 ` Erik Faye-Lund 1 sibling, 1 reply; 46+ messages in thread From: Junio C Hamano @ 2011-02-24 0:57 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > And then there's the issue that unlike the C patches these will not be > a no-op that'll be optimized away by the compiler. We'll be calling an > external program for displaying messages. While this is a trivial cost > on Unix (especially in the context we're using it, i.e. not in tight > loops) it's more expensive on Windows. > > I don't see any way to deal with that short of implementing some > pre-processor, but I think the cost is worth it, but others might > disagree of course. Count me one of the others then. Don't we already preprocess our scripts before installing anyway? > Anyway, I can submit these patches (around 53) real soon, or wait > until the current series settles. It's the same to me, which would you > prefer? One thing at a time is of course preferred. I actually want to stagger the current 70+ patches into two batches. Have the first handful, after polishing them really shiny, merged to master, and possibly rebase topics that are only in 'pu' and that are not meant for 'maint' on top of the result (so that they can use _() in newly added messages), and then merge "mark strings in git-foo with _()" patches in. I suspect it won't be the same to you. At least, it shouldn't. Other topics that fix real bugs or add features continue to trickle down from 'next' to 'master' and you would need to re-roll what you have today. If you wait (or if we have you wait) for too long, the re-roll would become just as unpleasant as my merging the i18n topic to 'pu'. > Warning: you are leaving 1 commit behind that are not connected to > any of your branches: > > For the singular this should be "1 commit behind which is not > corrected to any of your branches". Heh, thanks. I would think "s/ that are /, /" would fix it rather nicely. > ... but then > again that would confuse the sort of users that need this the most. That is exactly why I phrased it this way. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 0:57 ` Junio C Hamano @ 2011-02-24 1:37 ` Ævar Arnfjörð Bjarmason 2011-02-24 1:59 ` Junio C Hamano 0 siblings, 1 reply; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 1:37 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Thu, Feb 24, 2011 at 01:57, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> And then there's the issue that unlike the C patches these will not be >> a no-op that'll be optimized away by the compiler. We'll be calling an >> external program for displaying messages. While this is a trivial cost >> on Unix (especially in the context we're using it, i.e. not in tight >> loops) it's more expensive on Windows. >> >> I don't see any way to deal with that short of implementing some >> pre-processor, but I think the cost is worth it, but others might >> disagree of course. > > Count me one of the others then. Don't we already preprocess our scripts > before installing anyway? Yes, but only for simple variable substitution. Substituting all the gettext calls out when we're not compiling with them would be way more complex than what we do now, but possible. Anyway I'm not going to do it. >> Anyway, I can submit these patches (around 53) real soon, or wait >> until the current series settles. It's the same to me, which would you >> prefer? > > One thing at a time is of course preferred. I'll wait then. No point in submitting them if you won't be merging them down until the other things cool down. > I actually want to stagger the current 70+ patches into two batches. Have > the first handful, after polishing them really shiny, merged to master, > and possibly rebase topics that are only in 'pu' and that are not meant > for 'maint' on top of the result (so that they can use _() in newly added > messages), and then merge "mark strings in git-foo with _()" patches in. > > I suspect it won't be the same to you. At least, it shouldn't. > > Other topics that fix real bugs or add features continue to trickle down > from 'next' to 'master' and you would need to re-roll what you have today. > If you wait (or if we have you wait) for too long, the re-roll would > become just as unpleasant as my merging the i18n topic to 'pu'. Right, once we're confident (which at least I am) that the first few patches really are no-op's I think it would be easier for everyone to merge them down sooner than later. >> Warning: you are leaving 1 commit behind that are not connected to >> any of your branches: >> >> For the singular this should be "1 commit behind which is not >> corrected to any of your branches". > > Heh, thanks. I would think "s/ that are /, /" would fix it rather > nicely. s/corrected/connected/ also :) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 1:37 ` Ævar Arnfjörð Bjarmason @ 2011-02-24 1:59 ` Junio C Hamano 2011-02-24 2:07 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 46+ messages in thread From: Junio C Hamano @ 2011-02-24 1:59 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Jonathan Nieder, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Right, once we're confident (which at least I am) that the first few > patches really are no-op's I think it would be easier for everyone to > merge them down sooner than later. We three are pretty much on the same page, then. Two minor points are what to do with the "#" prefix in "# POISON #" and namespace contamination, but other than these I don't think there is anything controversial. >>> Warning: you are leaving 1 commit behind that are not connected to >>> any of your branches: >>> >>> For the singular this should be "1 commit behind which is not >>> corrected to any of your branches". >> >> Heh, thanks. I would think "s/ that are /, /" would fix it rather >> nicely. > > s/corrected/connected/ also :) That typo appears only in _your_ version, not mine ;-). ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 1:59 ` Junio C Hamano @ 2011-02-24 2:07 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 2:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: Jonathan Nieder, git On Thu, Feb 24, 2011 at 02:59, Junio C Hamano <gitster@pobox.com> wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> Right, once we're confident (which at least I am) that the first few >> patches really are no-op's I think it would be easier for everyone to >> merge them down sooner than later. > > We three are pretty much on the same page, then. > > Two minor points are what to do with the "#" prefix in "# POISON #" The root cause of this is solved in a patch by Jonathan: https://github.com/avar/git/commit/da15d84430e5f9b68125f6ddad61f66251c991ac I thought it was better to submit it later once this was in. The current behavior is harmless hack that we're aware of. > namespace contamination, You mean just _ being propagated everywhere? That seems harmless. > but other than these I don't think there is anything controversial. Great. >> s/corrected/connected/ also :) > > That typo appears only in _your_ version, not mine ;-). Indeed. I was amending my suggestion, not your original :) ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 0:32 ` Ævar Arnfjörð Bjarmason 2011-02-24 0:57 ` Junio C Hamano @ 2011-02-24 9:45 ` Erik Faye-Lund 2011-02-24 17:47 ` Ævar Arnfjörð Bjarmason 2011-02-25 9:05 ` Kirill Smelkov 1 sibling, 2 replies; 46+ messages in thread From: Erik Faye-Lund @ 2011-02-24 9:45 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git, msysGit On Thu, Feb 24, 2011 at 1:32 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > And then there's the issue that unlike the C patches these will not be > a no-op that'll be optimized away by the compiler. We'll be calling an > external program for displaying messages. While this is a trivial cost > on Unix (especially in the context we're using it, i.e. not in tight > loops) it's more expensive on Windows. > Ouch. I remember this being brought up earlier, but I just assumed it had been fixed somehow. The shell-scripts are already pretty slow on Windows, and the overhead of starting a new process here is quite significant. It sounds to me like we should revert these patches in msysGit, at least until there's some actual translations in place (and that time actually goes into something useful)... > I don't see any way to deal with that short of implementing some > pre-processor, but I think the cost is worth it, but others might > disagree of course. > I'm not so sure. This is mostly a problem with the no-op version on Windows (due to the slow process-startup there), but I think Git for Windows probably wants to have i18n support in it's distribution as it strives to be the canonical Git-distribution for Windows. But if we do, there's nothing to optimize. There's no no-op-stuff, and we need to spend that time getting translations. It might be that some people that build Git for Windows themselves and know that they don't want a translated Git could benefit from a pre-processor, but I'm not so sure. Translated strings occur when there's communication going on between Git and the user, and then we're some times waiting for user-input, and even when we aren't it should be relatively few messages (unless verbose flags are turned on, which isn't an important use-case performance-wise to me). > Anyway, I can submit these patches (around 53) real soon, or wait > until the current series settles. It's the same to me, which would you > prefer? > If we're going to revert these patches in 4msysgit.git, then I can imagine that the process becomes very awkward and error-prone if we're going to diverge for a long time. So I think it'd make more sense for us (the msysgit-developers, that is) if it was merged together with the first translations. But that might be sub-optimal for the rest of you guys. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 9:45 ` Erik Faye-Lund @ 2011-02-24 17:47 ` Ævar Arnfjörð Bjarmason 2011-02-24 22:39 ` Jonathan Nieder 2011-02-25 1:45 ` [msysGit] " Johannes Schindelin 2011-02-25 9:05 ` Kirill Smelkov 1 sibling, 2 replies; 46+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2011-02-24 17:47 UTC (permalink / raw) To: kusmabite; +Cc: Junio C Hamano, git, msysGit On Thu, Feb 24, 2011 at 10:45, Erik Faye-Lund <kusmabite@gmail.com> wrote: > On Thu, Feb 24, 2011 at 1:32 AM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> And then there's the issue that unlike the C patches these will not be >> a no-op that'll be optimized away by the compiler. We'll be calling an >> external program for displaying messages. While this is a trivial cost >> on Unix (especially in the context we're using it, i.e. not in tight >> loops) it's more expensive on Windows. >> > > Ouch. I remember this being brought up earlier, but I just assumed it > had been fixed somehow. The shell-scripts are already pretty slow on > Windows, and the overhead of starting a new process here is quite > significant. > > It sounds to me like we should revert these patches in msysGit, at > least until there's some actual translations in place (and that time > actually goes into something useful)... IIRC last time this was discussed I asked whether the size of the binary mattered for execution startup time (i.e. more so than on Unix). We're only invoking printf(1) and git-sh-i18n--envsubst, both of which only are (or only need to be) linked to libc. It would also be interesting to have some real world benchmarks on Windows with and without this series, maybe it won't be so bad. I think in the long term we probably want to rewrite the remaining *.sh programs in C anyway. >> I don't see any way to deal with that short of implementing some >> pre-processor, but I think the cost is worth it, but others might >> disagree of course. >> > > I'm not so sure. This is mostly a problem with the no-op version on > Windows (due to the slow process-startup there), but I think Git for > Windows probably wants to have i18n support in it's distribution as it > strives to be the canonical Git-distribution for Windows. But if we > do, there's nothing to optimize. There's no no-op-stuff, and we need > to spend that time getting translations. > > It might be that some people that build Git for Windows themselves and > know that they don't want a translated Git could benefit from a > pre-processor, but I'm not so sure. Translated strings occur when > there's communication going on between Git and the user, and then > we're some times waiting for user-input, and even when we aren't it > should be relatively few messages (unless verbose flags are turned on, > which isn't an important use-case performance-wise to me). > >> Anyway, I can submit these patches (around 53) real soon, or wait >> until the current series settles. It's the same to me, which would you >> prefer? >> > > If we're going to revert these patches in 4msysgit.git, then I can > imagine that the process becomes very awkward and error-prone if we're > going to diverge for a long time. So I think it'd make more sense for > us (the msysgit-developers, that is) if it was merged together with > the first translations. But that might be sub-optimal for the rest of > you guys. If it comes to that it'll be easier to have some perl script that converts C<"$(eval_gettext "foobar: \$whatever")"> back to C<"foobar: $whataver"> at build time than revert the patches. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 17:47 ` Ævar Arnfjörð Bjarmason @ 2011-02-24 22:39 ` Jonathan Nieder 2011-02-25 1:45 ` [msysGit] " Johannes Schindelin 1 sibling, 0 replies; 46+ messages in thread From: Jonathan Nieder @ 2011-02-24 22:39 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: kusmabite, Junio C Hamano, git, msysGit Ævar Arnfjörð Bjarmason wrote: > IIRC last time this was discussed I asked whether the size of the > binary mattered for execution startup time (i.e. more so than on > Unix). We're only invoking printf(1) and git-sh-i18n--envsubst, both > of which only are (or only need to be) linked to libc. printf is usually built in to the shell. I think if we're very careful about quoting shell metacharacters then we can get by using eval in place of envsubst. See the message that http://thread.gmane.org/gmane.comp.version-control.git/160396 is a reply to (which does not seem to have hit the ml; sorry about that). I hope preprocessing away the "eval" is not needed. :) > It would also be interesting to have some real world benchmarks on > Windows with and without this series, maybe it won't be so bad. Yes, e.g. timing from running the rebase tests in the testsuite might be interesting. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: [msysGit] Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 17:47 ` Ævar Arnfjörð Bjarmason 2011-02-24 22:39 ` Jonathan Nieder @ 2011-02-25 1:45 ` Johannes Schindelin 1 sibling, 0 replies; 46+ messages in thread From: Johannes Schindelin @ 2011-02-25 1:45 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: kusmabite, Junio C Hamano, git, msysGit [-- Attachment #1: Type: TEXT/PLAIN, Size: 334 bytes --] Hi, On Thu, 24 Feb 2011, Ævar Arnfjörð Bjarmason wrote: > I think in the long term we probably want to rewrite the remaining *.sh > programs in C anyway. Good luck with that, and also the Perl programs (it's not so much the technical side which I wish you luck with; the technical issues are straight-forward :-) Ciao, Dscho ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-24 9:45 ` Erik Faye-Lund 2011-02-24 17:47 ` Ævar Arnfjörð Bjarmason @ 2011-02-25 9:05 ` Kirill Smelkov 2011-02-25 11:11 ` Johannes Schindelin 1 sibling, 1 reply; 46+ messages in thread From: Kirill Smelkov @ 2011-02-25 9:05 UTC (permalink / raw) To: Johannes Schindelin Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit +Dscho On Thu, Feb 24, 2011 at 10:45:33AM +0100, Erik Faye-Lund wrote: > On Thu, Feb 24, 2011 at 1:32 AM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: > > And then there's the issue that unlike the C patches these will not be > > a no-op that'll be optimized away by the compiler. We'll be calling an > > external program for displaying messages. While this is a trivial cost > > on Unix (especially in the context we're using it, i.e. not in tight > > loops) it's more expensive on Windows. > > > > Ouch. I remember this being brought up earlier, but I just assumed it > had been fixed somehow. The shell-scripts are already pretty slow on > Windows, and the overhead of starting a new process here is quite > significant. Johannes, can we please try my patch[1] for msys.dll not to load user32.dll for every msys program (i.e. sh.exe too)? Combined together with 3 clipboard functions removal (details in [2]) from sh.exe, I bet this will result in significantly faster shell startup, configure runs, etc... This days I have lack of access to windows machines, only wine, so I desperately need someones help to at least first rebuild msys.dll. Thanks, Kirill [1] http://repo.or.cz/w/msys/kirr.git/commitdiff/f7d7efebd35e8e5bf6d685ff4f1197941984be04 [2] http://repo.or.cz/w/msys/kirr.git/commitdiff/a97bed5d22f9c05f39776d8ea7856db4ce572dc5 ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-25 9:05 ` Kirill Smelkov @ 2011-02-25 11:11 ` Johannes Schindelin 2011-02-25 19:24 ` Kirill Smelkov 0 siblings, 1 reply; 46+ messages in thread From: Johannes Schindelin @ 2011-02-25 11:11 UTC (permalink / raw) To: Kirill Smelkov Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit Hi, On Fri, 25 Feb 2011, Kirill Smelkov wrote: > Johannes, can we please try my patch[1] for msys.dll not to load > user32.dll for every msys program (i.e. sh.exe too)? Combined together > with 3 clipboard functions removal (details in [2]) from sh.exe, I bet > this will result in significantly faster shell startup, configure runs, > etc... > > This days I have lack of access to windows machines, only wine, so I > desperately need someones help to at least first rebuild msys.dll. > > [1] http://repo.or.cz/w/msys/kirr.git/commitdiff/f7d7efebd35e8e5bf6d685ff4f1197941984be04 > [2] http://repo.or.cz/w/msys/kirr.git/commitdiff/a97bed5d22f9c05f39776d8ea7856db4ce572dc5 Three things: [2] does not apply cleanly, so I am still desperately trying to find some time to finish it off (the patches are to msys.git, not the 'msys' branch of msysgit.git, so I had to find a quarter an hour in order to put them there in the first place, taking up all the Git time budget I wanted to allow myself for this week). Further, I think that my beloved Shift+Insert will no longer work with your [2]. And lastly, in [2] you claim that you cross-built msys-1.0.dll. I would like to have a script doing that in msysgit.git. Ciao, Dscho ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-25 11:11 ` Johannes Schindelin @ 2011-02-25 19:24 ` Kirill Smelkov 2011-02-25 21:54 ` Johannes Schindelin 0 siblings, 1 reply; 46+ messages in thread From: Kirill Smelkov @ 2011-02-25 19:24 UTC (permalink / raw) To: Johannes Schindelin Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit On Fri, Feb 25, 2011 at 12:11:59PM +0100, Johannes Schindelin wrote: > Hi, > > On Fri, 25 Feb 2011, Kirill Smelkov wrote: > > > Johannes, can we please try my patch[1] for msys.dll not to load > > user32.dll for every msys program (i.e. sh.exe too)? Combined together > > with 3 clipboard functions removal (details in [2]) from sh.exe, I bet > > this will result in significantly faster shell startup, configure runs, > > etc... > > > > This days I have lack of access to windows machines, only wine, so I > > desperately need someones help to at least first rebuild msys.dll. > > > > [1] http://repo.or.cz/w/msys/kirr.git/commitdiff/f7d7efebd35e8e5bf6d685ff4f1197941984be04 > > [2] http://repo.or.cz/w/msys/kirr.git/commitdiff/a97bed5d22f9c05f39776d8ea7856db4ce572dc5 For [1] please do git://repo.or.cz/msysgit/kirr.git ks/no-user32-in-msysdll # into msys Kirill Smelkov (1): msys.dll: Don't pull user32.dll & friends just to detect whether right alt should be used as meta ...n-t-pull-user32.dll-friends-just-to-detec.patch | 98 ++++++++++++++++++++ 1 files changed, 98 insertions(+), 0 deletions(-) create mode 100644 src/rt/patches/0011-msys.dll-Don-t-pull-user32.dll-friends-just-to-detec.patch > Three things: > > [2] does not apply cleanly, so I am still desperately trying to find some > time to finish it off (the patches are to msys.git, not the 'msys' branch > of msysgit.git, so I had to find a quarter an hour in order to put them > there in the first place, taking up all the Git time budget I wanted to > allow myself for this week). For [2] please do git pull git://repo.or.cz/msysgit/kirr.git ks/xser32.dll # into devel Kirill Smelkov (3): hack: xser32.dll -- Fake user32.dll like stub (for sh.exe not to load user32.dll) xser32.dll: Fake user32.dll like stub (for sh.exe not to load user32.dll) hack: sh.exe -- link to xser32.dll instead of user32.dll bin/sh.exe | Bin 567296 -> 567296 bytes bin/xser32.dll | Bin 0 -> 4373 bytes src/xser32/Makefile | 13 +++++++++++++ src/xser32/release.sh | 7 +++++++ src/xser32/xser32.c | 27 +++++++++++++++++++++++++++ 5 files changed, 47 insertions(+), 0 deletions(-) create mode 100644 bin/xser32.dll create mode 100644 src/xser32/Makefile create mode 100644 src/xser32/release.sh create mode 100644 src/xser32/xser32.c > Further, I think that my beloved Shift+Insert will no longer work with > your [2]. Probably yes, but this is only an experiment to see whether sh.exe starts to be really faster. For proper solution we'll need to patch bash to use msys's /dev/clipboard, or link to user32 symbols lazily. > And lastly, in [2] you claim that you cross-built msys-1.0.dll. I would > like to have a script doing that in msysgit.git. This is in progress. Preliminary stuff (cross-built msys.dll no longer needs mingwm10.dll), is here: http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys Sorry, I hoped to finish 2a today, but have to run... Thanks, Kirill ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-25 19:24 ` Kirill Smelkov @ 2011-02-25 21:54 ` Johannes Schindelin 2011-02-26 11:07 ` Kirill Smelkov 0 siblings, 1 reply; 46+ messages in thread From: Johannes Schindelin @ 2011-02-25 21:54 UTC (permalink / raw) To: Kirill Smelkov Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit Hi, On Fri, 25 Feb 2011, Kirill Smelkov wrote: > On Fri, Feb 25, 2011 at 12:11:59PM +0100, Johannes Schindelin wrote: > > > On Fri, 25 Feb 2011, Kirill Smelkov wrote: > > > > > Johannes, can we please try my patch[1] for msys.dll not to load > > > user32.dll for every msys program (i.e. sh.exe too)? Combined > > > together with 3 clipboard functions removal (details in [2]) from > > > sh.exe, I bet this will result in significantly faster shell > > > startup, configure runs, etc... > > > > > > This days I have lack of access to windows machines, only wine, so I > > > desperately need someones help to at least first rebuild msys.dll. > > > > > > [1] http://repo.or.cz/w/msys/kirr.git/commitdiff/f7d7efebd35e8e5bf6d685ff4f1197941984be04 > > > [2] http://repo.or.cz/w/msys/kirr.git/commitdiff/a97bed5d22f9c05f39776d8ea7856db4ce572dc5 > > For [1] please do > > git://repo.or.cz/msysgit/kirr.git ks/no-user32-in-msysdll # into msys Too late. I already did the work. > > Three things: > > > > [2] does not apply cleanly, so I am still desperately trying to find some > > time to finish it off (the patches are to msys.git, not the 'msys' branch > > of msysgit.git, so I had to find a quarter an hour in order to put them > > there in the first place, taking up all the Git time budget I wanted to > > allow myself for this week). > > For [2] please do > > git pull git://repo.or.cz/msysgit/kirr.git ks/xser32.dll # into devel Again, too late. I already did the work. > > Further, I think that my beloved Shift+Insert will no longer work with > > your [2]. > > Probably yes, In my experiment after rebuilding msys-1.0.dll, it still works. > > And lastly, in [2] you claim that you cross-built msys-1.0.dll. I > > would like to have a script doing that in msysgit.git. > > This is in progress. Preliminary stuff (cross-built msys.dll no longer needs > mingwm10.dll), is here: > > http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr > http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys Thanks, I will try to find some time to test this next week. The problem for now is that when I time /share/msysGit/run-tests.sh, there is hardly any gain from your patches: Old: real 18m1.031s user 6m17.861s sys 19m25.257s New: real 17m54.500s user 6m12.319s sys 19m28.567s Ciao, Dscho ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-25 21:54 ` Johannes Schindelin @ 2011-02-26 11:07 ` Kirill Smelkov 2011-02-26 19:07 ` Kirill Smelkov 2011-02-27 14:28 ` Johannes Schindelin 0 siblings, 2 replies; 46+ messages in thread From: Kirill Smelkov @ 2011-02-26 11:07 UTC (permalink / raw) To: Johannes Schindelin Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit Hi, On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > Hi, > > On Fri, 25 Feb 2011, Kirill Smelkov wrote: > > > On Fri, Feb 25, 2011 at 12:11:59PM +0100, Johannes Schindelin wrote: > > > > > On Fri, 25 Feb 2011, Kirill Smelkov wrote: > > > > > > > Johannes, can we please try my patch[1] for msys.dll not to load > > > > user32.dll for every msys program (i.e. sh.exe too)? Combined > > > > together with 3 clipboard functions removal (details in [2]) from > > > > sh.exe, I bet this will result in significantly faster shell > > > > startup, configure runs, etc... > > > > > > > > This days I have lack of access to windows machines, only wine, so I > > > > desperately need someones help to at least first rebuild msys.dll. > > > > > > > > [1] http://repo.or.cz/w/msys/kirr.git/commitdiff/f7d7efebd35e8e5bf6d685ff4f1197941984be04 > > > > [2] http://repo.or.cz/w/msys/kirr.git/commitdiff/a97bed5d22f9c05f39776d8ea7856db4ce572dc5 > > > > For [1] please do > > > > git://repo.or.cz/msysgit/kirr.git ks/no-user32-in-msysdll # into msys > > Too late. I already did the work. Sorry for the confision. This one is ok to stay the way as you did it. > > > > Three things: > > > > > > [2] does not apply cleanly, so I am still desperately trying to find some > > > time to finish it off (the patches are to msys.git, not the 'msys' branch > > > of msysgit.git, so I had to find a quarter an hour in order to put them > > > there in the first place, taking up all the Git time budget I wanted to > > > allow myself for this week). > > > > For [2] please do > > > > git pull git://repo.or.cz/msysgit/kirr.git ks/xser32.dll # into devel > > Again, too late. I already did the work. Sorry again, but as done in my ks/xser32.dll this is not tied to msys, so maybe better please pull my branch into devel instead? > > > Further, I think that my beloved Shift+Insert will no longer work with > > > your [2]. > > > > Probably yes, > > In my experiment after rebuilding msys-1.0.dll, it still works. xser32.dll has nothing to do with msys - it's just a fake stub for sh.exe. Before testing, have you "patched" sh.exe the way it is done in my ks/xser32.dll (http://repo.or.cz/w/msysgit/kirr.git/commit/9d952c74a52f577b2d16d4e4a489541a8fa7fbbd) ? If not, I'm not surprised it still works :) > > > And lastly, in [2] you claim that you cross-built msys-1.0.dll. I > > > would like to have a script doing that in msysgit.git. > > > > This is in progress. Preliminary stuff (cross-built msys.dll no longer needs > > mingwm10.dll), is here: > > > > http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr > > http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys > > Thanks, I will try to find some time to test this next week. Please don't - it does not build out of the box from msysgit yet. I just wanted to show it is not staying stale. When it is finished, I'll let you know. > The problem for now is that when I time /share/msysGit/run-tests.sh, there > is hardly any gain from your patches: > > Old: > > > real 18m1.031s > user 6m17.861s > sys 19m25.257s > > New: > > real 17m54.500s > user 6m12.319s > sys 19m28.567s Did you patch sh.exe to link to xser32.dll instead of user32.dll? This is important because even with rebuilt msys.dll, original sh.exe still links to user32.dll and this hides all the effort put into stripping user32.dll from msys.dll. Also I can't say for sure (hope yet) how sh-intensitive git tests are, but at least running configure for say gettext or whatever should be visibly faster, at least on wine. > > Ciao, > Dscho Thanks, Kirill ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 11:07 ` Kirill Smelkov @ 2011-02-26 19:07 ` Kirill Smelkov 2011-02-26 19:58 ` cross-compiling msys-1.0.dll, was " Johannes Schindelin 2011-02-27 14:28 ` Johannes Schindelin 1 sibling, 1 reply; 46+ messages in thread From: Kirill Smelkov @ 2011-02-26 19:07 UTC (permalink / raw) To: Johannes Schindelin Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit On Sat, Feb 26, 2011 at 02:07:40PM +0300, Kirill Smelkov wrote: > Hi, > > On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > > > > And lastly, in [2] you claim that you cross-built msys-1.0.dll. I > > > > would like to have a script doing that in msysgit.git. > > > > > > This is in progress. Preliminary stuff (cross-built msys.dll no longer needs > > > mingwm10.dll), is here: > > > > > > http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr > > > http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys > > > > Thanks, I will try to find some time to test this next week. > > Please don't - it does not build out of the box from msysgit yet. I just > wanted to show it is not staying stale. When it is finished, I'll let > you know. Done. Please do git pull git://repo.or.cz/msysgit/kirr.git ks/crossmsys # into msys to receive the following updates: Kirill Smelkov (3): msys: My patches to fix cross-compilation of msys.dll src/rt/release.sh: Move under-msys specific code into it's own function src/rt/release.sh: Teach it to cross-compile msys.dll on linux Thanks beforehand, Kirill [ Diffstat and cumulative patch also follow: ] .../0012-newlib-Fix-build-in-ctype_.c.patch | 43 +++++ ...p-cygwin-wsock_started-should-be-extern-C.patch | 33 ++++ ...nsup-cygwin-reent_data-should-be-extern-C.patch | 33 ++++ ...in-_sys_errlist-and-_sys_nerr-should-have.patch | 42 +++++ ...in-Don-t-use-non-lvalue-assignment-in-cyg.patch | 36 ++++ ...in-Don-t-use-non-lvalue-assgnment-in-dcrt.patch | 47 +++++ ...cygwin-struct-LUID-has-no-member-QuadPart.patch | 39 +++++ ...in-Fix-asm-constraint-for-_impure_ptr-err.patch | 38 ++++ ...in-Add-volatile-qualifier-to-ilock-in-win.patch | 70 ++++++++ ...in-call_signal_handler_now-alias-keeping-.patch | 48 +++++ ...in-std_dll_init-and-wsock_init-should-not.patch | 60 +++++++ ...-cygwin-Build-with-fno-threadsafe-statics.patch | 45 +++++ ...24-winsup-cygwin-Avoid-libstdc-dependency.patch | 103 +++++++++++ ...in-Strip-new-msys-1.0.dll-when-cross-comp.patch | 94 ++++++++++ ...in-cygrun.exe-build-depends-on-libmsys-1..patch | 32 ++++ ...in-cyrun.exe-should-be-built-with-nostdli.patch | 53 ++++++ src/rt/release.sh | 180 +++++++++++++++++--- 17 files changed, 974 insertions(+), 22 deletions(-) create mode 100644 src/rt/patches/0012-newlib-Fix-build-in-ctype_.c.patch create mode 100644 src/rt/patches/0013-winsup-cygwin-wsock_started-should-be-extern-C.patch create mode 100644 src/rt/patches/0014-winsup-cygwin-reent_data-should-be-extern-C.patch create mode 100644 src/rt/patches/0015-winsup-cygwin-_sys_errlist-and-_sys_nerr-should-have.patch create mode 100644 src/rt/patches/0016-winsup-cygwin-Don-t-use-non-lvalue-assignment-in-cyg.patch create mode 100644 src/rt/patches/0017-winsup-cygwin-Don-t-use-non-lvalue-assgnment-in-dcrt.patch create mode 100644 src/rt/patches/0018-winsup-cygwin-struct-LUID-has-no-member-QuadPart.patch create mode 100644 src/rt/patches/0019-winsup-cygwin-Fix-asm-constraint-for-_impure_ptr-err.patch create mode 100644 src/rt/patches/0020-winsup-cygwin-Add-volatile-qualifier-to-ilock-in-win.patch create mode 100644 src/rt/patches/0021-winsup-cygwin-call_signal_handler_now-alias-keeping-.patch create mode 100644 src/rt/patches/0022-winsup-cygwin-std_dll_init-and-wsock_init-should-not.patch create mode 100644 src/rt/patches/0023-winsup-cygwin-Build-with-fno-threadsafe-statics.patch create mode 100644 src/rt/patches/0024-winsup-cygwin-Avoid-libstdc-dependency.patch create mode 100644 src/rt/patches/0025-winsup-cygwin-Strip-new-msys-1.0.dll-when-cross-comp.patch create mode 100644 src/rt/patches/0026-winsup-cygwin-cygrun.exe-build-depends-on-libmsys-1..patch create mode 100644 src/rt/patches/0027-winsup-cygwin-cyrun.exe-should-be-built-with-nostdli.patch diff --git a/src/rt/patches/0012-newlib-Fix-build-in-ctype_.c.patch b/src/rt/patches/0012-newlib-Fix-build-in-ctype_.c.patch new file mode 100644 index 0000000..77fb1e0 --- /dev/null +++ b/src/rt/patches/0012-newlib-Fix-build-in-ctype_.c.patch @@ -0,0 +1,43 @@ +From 39dade0de1e56cb226a77cc3d9ab67c529895db6 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@landau.phys.spbu.ru> +Date: Sun, 13 Feb 2011 00:53:32 +0300 +Subject: [PATCH] newlib: Fix build in ctype_.c +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + + objs/../libc/ctype/ctype_.c:88: error: ‘_ctype_’ aliased to undefined symbol ‘_ctype_b+127’ + +gcc4 refuses to take expression as alias target: + + http://www.mail-archive.com/gcc%40gcc.gnu.org/msg01613.html + +So've cherry-hand-picked related patch by Cygwin's Corinna Vinschen +<vinschen@redhat.com> from here: + + http://cygwin.com/ml/newlib/2005/msg00189.html + +Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru> +--- + msys/rt/src/newlib/libc/ctype/ctype_.c | 5 ++++- + 1 files changed, 4 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/newlib/libc/ctype/ctype_.c b/msys/rt/src/newlib/libc/ctype/ctype_.c +index 54f92d9..863ec60 100644 +--- a/msys/rt/src/newlib/libc/ctype/ctype_.c ++++ b/msys/rt/src/newlib/libc/ctype/ctype_.c +@@ -85,7 +85,10 @@ static _CONST char _ctype_b[128 + 256] = { + }; + + #if defined(__CYGWIN__) || defined(__MSYS__) +-extern _CONST char __declspec(dllexport) _ctype_[1 + 256] __attribute__ ((alias ("_ctype_b+127"))); ++__asm__ ( ++ ".data \n\t" ++ ".globl __ctype_ \n\t" ++ ".set __ctype_,__ctype_b+127"); + _CONST char __declspec(dllexport) *__ctype_ptr = _ctype_b + 128; + #else + extern _CONST char _ctype_[1 + 256] __attribute__ ((alias ("_ctype_b+127"))); +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0013-winsup-cygwin-wsock_started-should-be-extern-C.patch b/src/rt/patches/0013-winsup-cygwin-wsock_started-should-be-extern-C.patch new file mode 100644 index 0000000..a7b3870 --- /dev/null +++ b/src/rt/patches/0013-winsup-cygwin-wsock_started-should-be-extern-C.patch @@ -0,0 +1,33 @@ +From 061bbbde0f631181579c12c6fb7f7f68e7118fdc Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 18:16:30 +0300 +Subject: [PATCH] winsup/cygwin: wsock_started should be extern "C" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... autoload.cc + .../winsup.h:240: error: previous declaration of ‘bool wsock_started’ with ‘C++’ linkage + .../autoload.cc:254: error: conflicts with new declaration with ‘C’ linkage + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/winsup.h | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/winsup.h b/msys/rt/src/winsup/cygwin/winsup.h +index 162c586..ecc5f54 100644 +--- a/msys/rt/src/winsup/cygwin/winsup.h ++++ b/msys/rt/src/winsup/cygwin/winsup.h +@@ -237,7 +237,7 @@ ssize_t check_iovec_for_write (const struct iovec *, int) __attribute__ ((regpar + #define set_winsock_errno() __set_winsock_errno (__FUNCTION__, __LINE__) + void __set_winsock_errno (const char *fn, int ln) __attribute__ ((regparm(2))); + +-extern bool wsock_started; ++extern "C" bool wsock_started; + + /* Printf type functions */ + extern "C" void __api_fatal (const char *, ...) __attribute__ ((noreturn)); +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0014-winsup-cygwin-reent_data-should-be-extern-C.patch b/src/rt/patches/0014-winsup-cygwin-reent_data-should-be-extern-C.patch new file mode 100644 index 0000000..1213e59 --- /dev/null +++ b/src/rt/patches/0014-winsup-cygwin-reent_data-should-be-extern-C.patch @@ -0,0 +1,33 @@ +From 6d8b0795ed154d975f89e9f5f8e27787af4bfcdd Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 18:19:30 +0300 +Subject: [PATCH] winsup/cygwin: reent_data should be extern "C" +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... dcrt0.cc + .../perthread.h:16: error: previous declaration of ‘_reent reent_data’ with ‘C++’ linkage + .../dcrt0.cc:91: error: conflicts with new declaration with ‘C’ linkage + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/perthread.h | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/perthread.h b/msys/rt/src/winsup/cygwin/perthread.h +index 185cc04..738a301 100644 +--- a/msys/rt/src/winsup/cygwin/perthread.h ++++ b/msys/rt/src/winsup/cygwin/perthread.h +@@ -13,7 +13,7 @@ details. */ + #define PTMAGIC 0x77366377 + + struct _reent; +-extern struct _reent reent_data; ++extern "C" struct _reent reent_data; + + extern DWORD *__stackbase __asm__ ("%fs:4"); + +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0015-winsup-cygwin-_sys_errlist-and-_sys_nerr-should-have.patch b/src/rt/patches/0015-winsup-cygwin-_sys_errlist-and-_sys_nerr-should-have.patch new file mode 100644 index 0000000..cdf4009 --- /dev/null +++ b/src/rt/patches/0015-winsup-cygwin-_sys_errlist-and-_sys_nerr-should-have.patch @@ -0,0 +1,42 @@ +From e608cceda43c4e7d6dd6829dd3ff1d0d6e0734e5 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 18:22:41 +0300 +Subject: [PATCH] winsup/cygwin: _sys_errlist and _sys_nerr should have external linkage +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... errno.cc + .../errno.cc:148: error: external linkage required for symbol ‘_sys_errlist’ because of ‘dllexport’ attribute + .../errno.cc:290: error: external linkage required for symbol ‘_sys_nerr’ because of ‘dllexport’ attribute + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/errno.cc | 4 ++-- + 1 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/errno.cc b/msys/rt/src/winsup/cygwin/errno.cc +index 89e25ff..e4a4a81 100644 +--- a/msys/rt/src/winsup/cygwin/errno.cc ++++ b/msys/rt/src/winsup/cygwin/errno.cc +@@ -145,7 +145,7 @@ seterrno (const char *file, int line) + + extern char *_user_strerror _PARAMS ((int)); + +-const NO_COPY char __declspec(dllexport) * const _sys_errlist[]= ++extern const NO_COPY char __declspec(dllexport) * const _sys_errlist[]= + { + /* NOERROR 0 */ "No error", + /* EPERM 1 */ "Not super-user", +@@ -287,7 +287,7 @@ const NO_COPY char __declspec(dllexport) * const _sys_errlist[]= + /* ECASECLASH 137 */ "Filename exists with different case" + }; + +-int const NO_COPY __declspec(dllexport) _sys_nerr = ++extern int const NO_COPY __declspec(dllexport) _sys_nerr = + sizeof (_sys_errlist) / sizeof (_sys_errlist[0]); + + /* FIXME: Why is strerror() a long switch and not just: +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0016-winsup-cygwin-Don-t-use-non-lvalue-assignment-in-cyg.patch b/src/rt/patches/0016-winsup-cygwin-Don-t-use-non-lvalue-assignment-in-cyg.patch new file mode 100644 index 0000000..6ad498d --- /dev/null +++ b/src/rt/patches/0016-winsup-cygwin-Don-t-use-non-lvalue-assignment-in-cyg.patch @@ -0,0 +1,36 @@ +From f9962a12ce9e1b47c5df8719b7b6079a91244131 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 18:30:28 +0300 +Subject: [PATCH] winsup/cygwin: Don't use non-lvalue assignment in cygheap.cc +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +There was one non-lvalue assgnment in cygheap.cc, and gcc does not +support them for ages... + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... cygheap.cc + .../cygheap.cc: In function ‘void* _csbrk(int)’: + .../cygheap.cc:169: error: lvalue required as left operand of assignment + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/cygheap.cc | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/cygheap.cc b/msys/rt/src/winsup/cygwin/cygheap.cc +index ac4d8fa..bdb93cc 100644 +--- a/msys/rt/src/winsup/cygwin/cygheap.cc ++++ b/msys/rt/src/winsup/cygwin/cygheap.cc +@@ -166,7 +166,7 @@ _csbrk (int sbs) + } + + lastheap = cygheap_max; +- (char *) cygheap_max += sbs; ++ cygheap_max = ((char *)cygheap_max) + sbs; + void *heapalign = (void *) pagetrunc (lastheap); + + if (!needalloc) +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0017-winsup-cygwin-Don-t-use-non-lvalue-assgnment-in-dcrt.patch b/src/rt/patches/0017-winsup-cygwin-Don-t-use-non-lvalue-assgnment-in-dcrt.patch new file mode 100644 index 0000000..45990bf --- /dev/null +++ b/src/rt/patches/0017-winsup-cygwin-Don-t-use-non-lvalue-assgnment-in-dcrt.patch @@ -0,0 +1,47 @@ +From 1448ec5e2ec6999122a60fd3f97c2ca89f660a4c Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 18:51:20 +0300 +Subject: [PATCH] winsup/cygwin: Don't use non-lvalue assgnment in dcrt0.cc +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +fork_info is defined as + + # define fork_info ((struct child_info_fork *)(si.lpReserved2)) + +so statement like + + fork_info = NULL; + +is non-lvalue assgnment: + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... dcrt0.cc + .../dcrt0.cc: In function ‘void _dll_crt0()’: + .../dcrt0.cc:958: error: lvalue required as left operand of assignment + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/dcrt0.cc | 4 ++-- + 1 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/dcrt0.cc b/msys/rt/src/winsup/cygwin/dcrt0.cc +index d3a67c7..6da4c0a 100644 +--- a/msys/rt/src/winsup/cygwin/dcrt0.cc ++++ b/msys/rt/src/winsup/cygwin/dcrt0.cc +@@ -955,10 +955,10 @@ _dll_crt0 () + } + default: + #if defined (__MSYS__) +- fork_info = NULL; ++ si.lpReserved2 = NULL; /* <-- means fork_info=NULL */ + #else /* !__MSYS__ */ + if (_cygwin_testing) +- fork_info = NULL; ++ si.lpReserved2 = NULL; /* <-- means fork_info=NULL */ + else if ((fork_info->type & PROC_MAGIC_MASK) == PROC_MAGIC_GENERIC) + api_fatal ("conflicting versions of cygwin1.dll detected. Use only the most recent version.\n"); + #endif /* !__MSYS__ */ +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0018-winsup-cygwin-struct-LUID-has-no-member-QuadPart.patch b/src/rt/patches/0018-winsup-cygwin-struct-LUID-has-no-member-QuadPart.patch new file mode 100644 index 0000000..d735178 --- /dev/null +++ b/src/rt/patches/0018-winsup-cygwin-struct-LUID-has-no-member-QuadPart.patch @@ -0,0 +1,39 @@ +From 7d49eefd97fe0276ff493580b749107713af54c0 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 18:39:17 +0300 +Subject: [PATCH] winsup/cygwin: struct LUID has no member QuadPart +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... security.cc + ../security.cc: In function ‘BOOL get_group_sidlist(const char*, cygsidlist&, cygsid&, cygsid&, _TOKEN_GROUPS*, LUID, int&)’: + ../security.cc:507: error: ‘struct LUID’ has no member named ‘QuadPart’ + +According to MSDN[1] there are only .LowPart and .HighPart in LUID, and +e.g. one wine patch[2] suggests how it should be redone. + +[1] http://msdn.microsoft.com/en-us/library/aa379261(v=vs.85).aspx +[2] http://www.winehq.org/pipermail/wine-patches/2001-November/001400.html + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/security.cc | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/security.cc b/msys/rt/src/winsup/cygwin/security.cc +index 7f0080e..5d48248 100644 +--- a/msys/rt/src/winsup/cygwin/security.cc ++++ b/msys/rt/src/winsup/cygwin/security.cc +@@ -504,7 +504,7 @@ get_group_sidlist (const char *logonserver, cygsidlist &grp_list, + grp_list += well_known_interactive_sid; + grp_list += well_known_authenticated_users_sid; + } +- if (auth_luid.QuadPart != 999) /* != SYSTEM_LUID */ ++ if ( ! (auth_luid.HighPart == 0 && auth_luid.LowPart == 999) ) /* != SYSTEM_LUID */ + { + char buf[64]; + __small_sprintf (buf, "S-1-5-5-%u-%u", auth_luid.HighPart, +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0019-winsup-cygwin-Fix-asm-constraint-for-_impure_ptr-err.patch b/src/rt/patches/0019-winsup-cygwin-Fix-asm-constraint-for-_impure_ptr-err.patch new file mode 100644 index 0000000..348252b --- /dev/null +++ b/src/rt/patches/0019-winsup-cygwin-Fix-asm-constraint-for-_impure_ptr-err.patch @@ -0,0 +1,38 @@ +From 013548ec033024245161685e93a3467d6bd2dbc0 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 19:50:48 +0300 +Subject: [PATCH] winsup/cygwin: Fix asm constraint for (&_impure_ptr->errno) + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... exceptions.cc + .../exceptions.cc:1234: error: memory input 1 is not directly addressable + +It was about "m" (&_impure_ptr->_errno). Earlier gcc accepted this, but +somehow world changed. Tell gcc the address can be also placed in a +register to make it happy. + +Similiar issue resolved this way: + +http://coding.derkeiler.com/Archive/Assembler/alt.lang.asm/2008-11/msg00079.html +http://coding.derkeiler.com/Archive/Assembler/alt.lang.asm/2008-11/msg00082.html + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/exceptions.cc | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/exceptions.cc b/msys/rt/src/winsup/cygwin/exceptions.cc +index 94b3a0b..020c6d8 100644 +--- a/msys/rt/src/winsup/cygwin/exceptions.cc ++++ b/msys/rt/src/winsup/cygwin/exceptions.cc +@@ -1226,7 +1226,7 @@ _sigdelayed0: \n\ + popl %%eax \n\ + jmp *%%eax \n\ + __no_sig_end: \n\ +-" : "=m" (sigsave.sig) : "m" (&_impure_ptr->_errno), ++" : "=m" (sigsave.sig) : "rm" (&_impure_ptr->_errno), + "g" (sigsave.retaddr), "g" (sigsave.oldmask), "g" (sigsave.sig), + "g" (sigsave.func), "o" (pid_offset), "g" (sigsave.saved_errno), "g" (sigsave.newmask) + ); +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0020-winsup-cygwin-Add-volatile-qualifier-to-ilock-in-win.patch b/src/rt/patches/0020-winsup-cygwin-Add-volatile-qualifier-to-ilock-in-win.patch new file mode 100644 index 0000000..3b23139 --- /dev/null +++ b/src/rt/patches/0020-winsup-cygwin-Add-volatile-qualifier-to-ilock-in-win.patch @@ -0,0 +1,70 @@ +From 2fa5be857f3b026f245dd7a283de093809568f05 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Sat, 26 Feb 2011 14:38:42 +0300 +Subject: [PATCH] winsup/cygwin: Add volatile qualifier to ilock* in winbase.h +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +[ Cherry-picked from r1.14 in + http://cygwin.com/cgi-bin/cvsweb.cgi/src/winsup/cygwin/winbase.h?cvsroot=src ] + +This is needed in order to fix compilation with recent gcc/w32api: + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... thread.cc + .../thread.cc: In constructor ‘pthread_cond::pthread_cond(pthread_condattr*)’: + .../thread.cc:438: error: invalid conversion from ‘volatile LONG*’ to ‘long int*’ + .../thread.cc:438: error: initializing argument 1 of ‘long int ilockexch(long int*, long int)’ + .../thread.cc: In destructor ‘pthread_cond::~pthread_cond()’: + .../thread.cc:448: error: invalid conversion from ‘volatile LONG*’ to ‘long int*’ + .../thread.cc:448: error: initializing argument 1 of ‘long int ilockexch(long int*, long int)’ + [ and so on ... ] + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/winbase.h | 8 ++++---- + 1 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/winbase.h b/msys/rt/src/winsup/cygwin/winbase.h +index 351d320..1257781 100644 +--- a/msys/rt/src/winsup/cygwin/winbase.h ++++ b/msys/rt/src/winsup/cygwin/winbase.h +@@ -4,7 +4,7 @@ + #define _WINBASE2_H + + extern __inline__ long +-ilockincr (long *m) ++ilockincr (volatile long *m) + { + register int __res; + __asm__ __volatile__ ("\n\ +@@ -16,7 +16,7 @@ ilockincr (long *m) + } + + extern __inline__ long +-ilockdecr (long *m) ++ilockdecr (volatile long *m) + { + register int __res; + __asm__ __volatile__ ("\n\ +@@ -28,7 +28,7 @@ ilockdecr (long *m) + } + + extern __inline__ long +-ilockexch (long *t, long v) ++ilockexch (volatile long *t, long v) + { + register int __res; + __asm__ __volatile__ ("\n\ +@@ -39,7 +39,7 @@ ilockexch (long *t, long v) + } + + extern __inline__ long +-ilockcmpexch (long *t, long v, long c) ++ilockcmpexch (volatile long *t, long v, long c) + { + register int __res; + __asm__ __volatile__ ("\n\ +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0021-winsup-cygwin-call_signal_handler_now-alias-keeping-.patch b/src/rt/patches/0021-winsup-cygwin-call_signal_handler_now-alias-keeping-.patch new file mode 100644 index 0000000..096d757 --- /dev/null +++ b/src/rt/patches/0021-winsup-cygwin-call_signal_handler_now-alias-keeping-.patch @@ -0,0 +1,48 @@ +From 85e943cb4e6aeac844ee465017efa5474c85e046 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 20:37:09 +0300 +Subject: [PATCH] winsup/cygwin: call_signal_handler_now() alias-keeping breaks compilation +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + + i586-mingw32msvc-g++ -c -O2 -D__MSYS__ -MD -fbuiltin ... exceptions.cc + .../exceptions.cc:1148: error: ‘int call_signal_handler_now_dummy()’ aliased to undefined symbol ‘call_signal_handler_now’ + +Unbreak it by making call_signal_handler_now() non-static and commenting +the kludge. Hope it's ok. + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/exceptions.cc | 4 +++- + 1 files changed, 3 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/exceptions.cc b/msys/rt/src/winsup/cygwin/exceptions.cc +index 020c6d8..a2853e9 100644 +--- a/msys/rt/src/winsup/cygwin/exceptions.cc ++++ b/msys/rt/src/winsup/cygwin/exceptions.cc +@@ -1127,7 +1127,7 @@ events_terminate (void) + } + + extern "C" { +-static int __stdcall ++int __stdcall + call_signal_handler_now () + { + if (!sigsave.sig) +@@ -1142,10 +1142,12 @@ call_signal_handler_now () + sigdelayed0 (); + return sa_flags & SA_RESTART; + } ++#if 0 + /* This kludge seems to keep a copy of call_signal_handler_now around + even when compiling with -finline-functions. */ + static int __stdcall call_signal_handler_now_dummy () + __attribute__((alias ("call_signal_handler_now"))); ++#endif + }; + + int +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0022-winsup-cygwin-std_dll_init-and-wsock_init-should-not.patch b/src/rt/patches/0022-winsup-cygwin-std_dll_init-and-wsock_init-should-not.patch new file mode 100644 index 0000000..27441b4 --- /dev/null +++ b/src/rt/patches/0022-winsup-cygwin-std_dll_init-and-wsock_init-should-not.patch @@ -0,0 +1,60 @@ +From 655638fd57f15e48c59620b763fbdc85bc7793a4 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 20:16:45 +0300 +Subject: [PATCH] winsup/cygwin: std_dll_init() and wsock_init() should not be static + +Because they are referenced from assembly-made LoadDllPrime from +different from .text sections: + + <linking new-msys-1.0.dll> + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x0): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0xc): undefined reference to `wsock_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x17): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x2e): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x45): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x5c): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x73): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.ws2_32_info[_ws2_32_handle]+0x8a): more undefined references to `std_dll_init' follow + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0xc): undefined reference to `wsock_init' + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0x18): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0x30): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0x48): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0x60): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0x78): undefined reference to `std_dll_init' + autoload.o:autoload.cc:(.wsock32_info[_wsock32_handle]+0x90): more undefined references to `std_dll_init' follow + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/autoload.cc | 8 ++++---- + 1 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/autoload.cc b/msys/rt/src/winsup/cygwin/autoload.cc +index 67d0bc0..6f5a8f5 100644 +--- a/msys/rt/src/winsup/cygwin/autoload.cc ++++ b/msys/rt/src/winsup/cygwin/autoload.cc +@@ -201,8 +201,8 @@ union retchain + }; + + /* The standard DLL initialization routine. */ +-static long long std_dll_init () __asm__ ("std_dll_init") __attribute__ ((unused)); +-static long long ++long long std_dll_init () __asm__ ("std_dll_init") __attribute__ ((unused)); ++long long + std_dll_init () + { + HANDLE h; +@@ -241,9 +241,9 @@ std_dll_init () + } + + /* Initialization function for winsock stuff. */ +-static long long wsock_init () __asm__ ("wsock_init") __attribute__ ((unused, regparm(1))); ++long long wsock_init () __asm__ ("wsock_init") __attribute__ ((unused, regparm(1))); + bool NO_COPY wsock_started = 0; +-static long long ++long long + wsock_init () + { + static LONG NO_COPY here = -1L; +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0023-winsup-cygwin-Build-with-fno-threadsafe-statics.patch b/src/rt/patches/0023-winsup-cygwin-Build-with-fno-threadsafe-statics.patch new file mode 100644 index 0000000..023dd80 --- /dev/null +++ b/src/rt/patches/0023-winsup-cygwin-Build-with-fno-threadsafe-statics.patch @@ -0,0 +1,45 @@ +From bcf492ef90d866cced91f6fd8f57fd0b4cd05691 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 21:57:51 +0300 +Subject: [PATCH] winsup/cygwin: Build with -fno-threadsafe-statics + +msys is low-level enough to do it's own synchronisation when neccessary, +and also gcc3 does not seem to be implementing threadsafe statics, so +msys never used this mechanics. + +But when compiled with gcc4, threadsafe statics are default - this adds +to code size, and also pulls __cxa_guard_acquire and __cxa_guard_release +from libstdc++. + +Avoid it for compilers that understand threadsafe statics (gcc4). The +goal is to avoid libstdc++ dependency completly in the end. + +See also: + +http://lists.apple.com/archives/darwin-drivers/2005/May/msg00067.html +http://gcc.gnu.org/ml/libstdc++/2002-11/msg00279.html + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/Makefile.common | 5 +++++ + 1 files changed, 5 insertions(+), 0 deletions(-) + +diff --git a/msys/rt/src/winsup/Makefile.common b/msys/rt/src/winsup/Makefile.common +index 78595d2..798c374 100644 +--- a/msys/rt/src/winsup/Makefile.common ++++ b/msys/rt/src/winsup/Makefile.common +@@ -99,6 +99,11 @@ CPP_TARGET_INCLUDE:=/usr/include/c++/3.2.2/i686-pc-msys + COMPILE_CXX:=$(CXX) $c -nostdinc++ $(ALL_CXXFLAGS) -I$(GCC_INCLUDE) \ + -I$(CPP_INCLUDE) -I$(CPP_TARGET_INCLUDE) \ + -fno-rtti -fno-exceptions ++ ++# add -fno-threadsafe-statics if compiler supports it ++COMPILE_CXX+=$(shell $(CXX) -fno-threadsafe-statics -c -xc++ /dev/null >/dev/null 2>&1 \ ++ && echo -fno-threadsafe-statics) ++ + COMPILE_CC:=$(CC) $c -nostdinc $(ALL_CFLAGS) -I$(GCC_INCLUDE) + + vpath %.a $(cygwin_build):$(w32api_lib):$(newlib_build)/libc:$(newlib_build)/libm +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0024-winsup-cygwin-Avoid-libstdc-dependency.patch b/src/rt/patches/0024-winsup-cygwin-Avoid-libstdc-dependency.patch new file mode 100644 index 0000000..eb55fc0 --- /dev/null +++ b/src/rt/patches/0024-winsup-cygwin-Avoid-libstdc-dependency.patch @@ -0,0 +1,103 @@ +From dfd3d1291f744f0f9c5549a58f69c814cb980266 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Fri, 25 Feb 2011 22:04:06 +0300 +Subject: [PATCH] winsup/cygwin: Avoid libstdc++ dependency + +All we need from libstdc++ is operator new/delete, and also +__cxa_pure_virtual(). + +But if we are getting this stuff from libstdc++, the functions also +pulls lots of dependencies, e.g. operator new calls std::terminate() if +it can't allocate memory, which links to exception-handling routines and +other symbols from non-msys C runtime: + + <linking msys.dll (with libstdc++)> + TODO + +Avoid it. + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/Makefile.in | 3 +- + msys/rt/src/winsup/cygwin/supcc.cc | 48 +++++++++++++++++++++++++++++++++ + 2 files changed, 50 insertions(+), 1 deletions(-) + create mode 100644 msys/rt/src/winsup/cygwin/supcc.cc + +diff --git a/msys/rt/src/winsup/cygwin/Makefile.in b/msys/rt/src/winsup/cygwin/Makefile.in +index ef0ae9e..050fc8a 100644 +--- a/msys/rt/src/winsup/cygwin/Makefile.in ++++ b/msys/rt/src/winsup/cygwin/Makefile.in +@@ -131,6 +131,7 @@ DLL_OFILES:=assert.o autoload.o \ + smallprint.o spawn.o strace.o strsep.o sync.o syscalls.o sysconf.o \ + syslog.o termios.o thread.o times.o tty.o uinfo.o uname.o wait.o \ + window.o \ ++ supcc.o \ + $(EXTRA_DLL_OFILES) $(EXTRA_OFILES) $(MALLOC_OFILES) $(MT_SAFE_OBJECTS) + + GMON_OFILES:= gmon.o mcount.o profil.o +@@ -201,7 +202,7 @@ new-$(DLL_NAME): $(LDSCRIPT) $(DLL_OFILES) $(DEF_FILE) $(DLL_IMPORTS) $(LIBC) $( + $(CXX) $(CXXFLAGS) -nostdlib -Wl,-T$(firstword $^) -shared -o $@ \ + -e $(DLL_ENTRY) $(DEF_FILE) $(DLL_OFILES) version.o winver.o \ + $(DLL_IMPORTS) $(MALLOC_OBJ) $(LIBM) $(LIBC) \ +- -lstdc++ -lgcc -lshell32 -luuid -lkernel32 -lnetapi32 -luser32 ++ -lgcc -lshell32 -luuid -lkernel32 -lnetapi32 -luser32 + + dll_ofiles: $(DLL_OFILES) + +diff --git a/msys/rt/src/winsup/cygwin/supcc.cc b/msys/rt/src/winsup/cygwin/supcc.cc +new file mode 100644 +index 0000000..1d3d6b5 +--- /dev/null ++++ b/msys/rt/src/winsup/cygwin/supcc.cc +@@ -0,0 +1,48 @@ ++/* C++ supporting routines - minimal selected bits from libsupc++ for msys.dll ++ * to work without linking to libstdc++ ++ */ ++#include "winsup.h" ++#include <stdlib.h> ++ ++void * ++operator new (size_t size) ++{ ++ void *p; ++ ++ p = malloc(size); ++ /* TODO if !p - try __new_handler */ ++ ++ if (!p) ++ api_fatal("out of memory in `operator new()`"); ++ ++ return p; ++} ++ ++void ++operator delete (void *p) ++{ ++ if (p) ++ free(p); ++} ++ ++ ++void * ++operator new[] (size_t size) ++{ ++ return operator new(size); ++} ++ ++void ++operator delete[] (void *p) ++{ ++ operator delete (p); ++} ++ ++ ++ ++/* pure virtuals link to here */ ++extern "C" void ++__cxa_pure_virtual() ++{ ++ api_fatal("pure virtual method called"); ++} +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0025-winsup-cygwin-Strip-new-msys-1.0.dll-when-cross-comp.patch b/src/rt/patches/0025-winsup-cygwin-Strip-new-msys-1.0.dll-when-cross-comp.patch new file mode 100644 index 0000000..c840148 --- /dev/null +++ b/src/rt/patches/0025-winsup-cygwin-Strip-new-msys-1.0.dll-when-cross-comp.patch @@ -0,0 +1,94 @@ +From 907b5b7d60f82ca51792ee9f048b08f2f6510e84 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Sat, 26 Feb 2011 15:01:04 +0300 +Subject: [PATCH] winsup/cygwin: Strip new-msys-1.0.dll when cross-compiling + +Somehow mingw32 cross compiler on Debian (lenny and squeeze) produces +dll's with .debug_* sections with VirtualSize=0: + + $ objdump -h new-msys-1.0.dll + new-msys-1.0.dll: file format efi-app-ia32 + + Sections: + Idx Name Size VMA LMA File off Algn + 0 .debug_abbrev 000137f0 00000000 00000000 000007e0 2**0 + CONTENTS, READONLY, DEBUGGING + 1 .debug_info 0006f0b5 00000000 00000000 00013fe0 2**0 + CONTENTS, READONLY, DEBUGGING + 2 .debug_line 0001c3fb 00000000 00000000 000831e0 2**0 + CONTENTS, READONLY, DEBUGGING + 3 .debug_pubnames 0000303f 00000000 00000000 0009f5e0 2**0 + CONTENTS, READONLY, DEBUGGING + 4 .debug_frame 000056d0 00000000 00000000 000a27e0 2**2 + CONTENTS, READONLY, DEBUGGING + 5 .debug_loc 0001ba99 00000000 00000000 000a7fe0 2**0 + CONTENTS, READONLY, DEBUGGING + 6 .debug_aranges 00002da0 00000000 00000000 000c3be0 2**3 + CONTENTS, READONLY, DEBUGGING + 7 .debug_str 00000390 00000000 00000000 000c69e0 2**0 + CONTENTS, READONLY, DEBUGGING + 8 .debug_ranges 000022a0 00000000 00000000 000c6de0 2**0 + CONTENTS, READONLY, DEBUGGING + 9 .text 00081e30 71001000 71001000 000c9200 2**5 + CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA + 10 .data 00004958 71083000 71083000 0014b200 2**5 + CONTENTS, ALLOC, LOAD, DATA + 11 .wsock32_info 00000378 71088000 71088000 0014fc00 2**2 + CONTENTS, ALLOC, LOAD, DATA, LINK_ONCE_DISCARD (COMDAT _wsock32_handle 15549) + 12 .ws2_32_info 00000144 71089000 71089000 00150000 2**2 + CONTENTS, ALLOC, LOAD, DATA, LINK_ONCE_DISCARD (COMDAT _ws2_32_handle 14700) + + ... + +And wine refuses to load PE's with such sections: + + $ WINEDEBUG=module wine hellomsys.exe + ... + trace:module:load_native_dll Trying native dll L"Z:\\home\\kirr\\src\\tools\\git\\msys\\msys\\rt\\src\\winsup\\cygwin\\objs\\msys-1.0.dll" + trace:module:map_image mapped PE file at 0x71000000-0x71102000 + warn:module:map_image Section /4 too large (8f000000+14000/102000) + warn:module:load_dll Failed to load module L"msys-1.0.dll"; status=c000007b + err:module:import_dll Loading library msys-1.0.dll (which is needed by L"Z:\\...\\xmsys.exe") failed (error c000007b). + err:module:LdrInitializeThunk Main exe initialization for L"Z:\\...\\xmsys.exe" failed, status c0000135 + +Now, c000007b is ERROR_BAD_EXE_FORMAT (= STATUS_INVALID_IMAGE_FORMAT), +and the error is triggered in ntdll:map_image(): + +http://source.winehq.org/git/wine.git/?a=blob;f=dlls/ntdll/virtual.c;h=0913b714bed3e2f766c7e4f056335224316d52c3;hb=HEAD#l1217 + +because of + + if (!sec->Misc.VirtualSize) + map_size = ROUND_SIZE( 0, sec->SizeOfRawData ); + else + map_size = ROUND_SIZE( 0, sec->Misc.VirtualSize ); + +I'm not PE expert, and can't tell who is right, so to workaround +problems, let's just strip debuginfo... + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/Makefile.in | 7 +++++++ + 1 files changed, 7 insertions(+), 0 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/Makefile.in b/msys/rt/src/winsup/cygwin/Makefile.in +index 050fc8a..e8eafe6 100644 +--- a/msys/rt/src/winsup/cygwin/Makefile.in ++++ b/msys/rt/src/winsup/cygwin/Makefile.in +@@ -204,6 +204,13 @@ new-$(DLL_NAME): $(LDSCRIPT) $(DLL_OFILES) $(DEF_FILE) $(DLL_IMPORTS) $(LIBC) $( + $(DLL_IMPORTS) $(MALLOC_OBJ) $(LIBM) $(LIBC) \ + -lgcc -lshell32 -luuid -lkernel32 -lnetapi32 -luser32 + ++ifneq ($(findstring mingw32,$(CXX)),) ++ # wine loader does not like debug sections with VirtualSize 0 - so when ++ # cross-compiling, to workaround this problem, just strip them. XXX better use $$(STRIP) ++ $(patsubst %-g++,%-strip,$(CXX)) -g $@ ++endif ++ ++ + dll_ofiles: $(DLL_OFILES) + + $(LIBGMON_A): $(GMON_OFILES) $(GMON_START) +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0026-winsup-cygwin-cygrun.exe-build-depends-on-libmsys-1..patch b/src/rt/patches/0026-winsup-cygwin-cygrun.exe-build-depends-on-libmsys-1..patch new file mode 100644 index 0000000..6496696 --- /dev/null +++ b/src/rt/patches/0026-winsup-cygwin-cygrun.exe-build-depends-on-libmsys-1..patch @@ -0,0 +1,32 @@ +From 625a8d421d923b59d680b319ac2b6439eb46ecc6 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Sat, 26 Feb 2011 17:15:52 +0300 +Subject: [PATCH] winsup/cygwin: cygrun.exe build-depends on libmsys-1.0.dll.a + +Since we link to it we should tell make we need it, or else `make -j4` +breaks: + + i586-mingw32msvc-gcc -nodefaultlibs -o cygrun.exe cygrun.o -lgcc libmsys-1.0.dll.a -luser32 -lshell32 -lkernel32 + i586-mingw32msvc-gcc: libmsys-1.0.dll.a: No such file or directory + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/Makefile.in | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/Makefile.in b/msys/rt/src/winsup/cygwin/Makefile.in +index e8eafe6..a1cbf71 100644 +--- a/msys/rt/src/winsup/cygwin/Makefile.in ++++ b/msys/rt/src/winsup/cygwin/Makefile.in +@@ -225,7 +225,7 @@ winver_stamp: mkvers.sh include/cygwin/version.h winver.rc $(DLL_OFILES) + touch $@ && \ + $(COMPILE_CXX) -o version.o version.cc + +-cygrun.exe : cygrun.o ++cygrun.exe : cygrun.o $(LIB_NAME) + $(CC) -nodefaultlibs -o $@ $^ -lgcc $(LIB_NAME) -luser32 -lshell32 -lkernel32 + + #\f +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/patches/0027-winsup-cygwin-cyrun.exe-should-be-built-with-nostdli.patch b/src/rt/patches/0027-winsup-cygwin-cyrun.exe-should-be-built-with-nostdli.patch new file mode 100644 index 0000000..215f788 --- /dev/null +++ b/src/rt/patches/0027-winsup-cygwin-cyrun.exe-should-be-built-with-nostdli.patch @@ -0,0 +1,53 @@ +From cf59176197193d2256f13580b5a8518b5e66a9c0 Mon Sep 17 00:00:00 2001 +From: Kirill Smelkov <kirr@mns.spb.ru> +Date: Sat, 26 Feb 2011 17:19:54 +0300 +Subject: [PATCH] winsup/cygwin: cyrun.exe should be built with -nostdlib (not -nodefaultlibs) + +Because -nodefaultlibs, still pulls in compiler's crt bits. E.g. when +cross compiling: + + i586-mingw32msvc-gcc -nodefaultlibs -o cygrun.exe cygrun.o libmsys-1.0.dll.a -lgcc libmsys-1.0.dll.a -luser32 -lshell32 -lkernel32 + libmsys-1.0.dll.a(dncqs00563.o):(.text+0x0): multiple definition of `_atexit' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:/home/ron/devel/debian/mingw32-runtime/mingw32-runtime-3.13/build_dir/src/mingw-runtime-3.13-20070825-1/crt1.c:280: first defined here + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x13): undefined reference to `__imp___onexit' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x35): undefined reference to `___cpu_features_init' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x3a): undefined reference to `__fpreset' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x51): undefined reference to `__CRT_glob' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x62): undefined reference to `___getmainargs' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x67): undefined reference to `__CRT_fmode' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x73): undefined reference to `___p__fmode' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x80): undefined reference to `__pei386_runtime_relocator' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x8d): undefined reference to `___p__environ' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0xaa): undefined reference to `__cexit' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0xb8): undefined reference to `__imp___iob' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0xd8): undefined reference to `__CRT_fmode' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0xf1): undefined reference to `__CRT_fmode' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x11a): undefined reference to `__imp____set_app_type' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x13a): undefined reference to `__imp____set_app_type' + /usr/lib/gcc/i586-mingw32msvc/4.4.2/../../../../i586-mingw32msvc/lib/crt2.o:crt1.c:(.text+0x278): undefined reference to `__fpreset' + collect2: ld returned 1 exit status + +Since newlib/msys already have all what is needed, we can turn +-nodefaultlibs into -nostdlib, which avoid standard crt2.o as well. + +Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> +--- + msys/rt/src/winsup/cygwin/Makefile.in | 2 +- + 1 files changed, 1 insertions(+), 1 deletions(-) + +diff --git a/msys/rt/src/winsup/cygwin/Makefile.in b/msys/rt/src/winsup/cygwin/Makefile.in +index a1cbf71..393f2da 100644 +--- a/msys/rt/src/winsup/cygwin/Makefile.in ++++ b/msys/rt/src/winsup/cygwin/Makefile.in +@@ -226,7 +226,7 @@ winver_stamp: mkvers.sh include/cygwin/version.h winver.rc $(DLL_OFILES) + $(COMPILE_CXX) -o version.o version.cc + + cygrun.exe : cygrun.o $(LIB_NAME) +- $(CC) -nodefaultlibs -o $@ $^ -lgcc $(LIB_NAME) -luser32 -lshell32 -lkernel32 ++ $(CC) -nostdlib -o $@ $^ -lgcc $(LIB_NAME) -luser32 -lshell32 -lkernel32 + + #\f + +-- +1.7.4.1.48.g5673d + diff --git a/src/rt/release.sh b/src/rt/release.sh index a25f37a..c81f5e1 100644 --- a/src/rt/release.sh +++ b/src/rt/release.sh @@ -8,7 +8,26 @@ die () { cd "$(dirname "$0")" debug= -test "a$1" = "a--debug" && debug=t +cross= + +while test $# != 0 +do + case "$1" in + "--debug") + debug=t + ;; + + "--cross") + cross=t + ;; + *) + die "Unknown option $1" + ;; + esac + + shift +done + debug_clean= test "$debug" = "$(cat debug.txt 2>/dev/null)" || debug_clean=t echo "$debug" > debug.txt @@ -36,26 +55,143 @@ do i=$(($i+1)) done -test -f /bin/cc.exe || ln gcc.exe /bin/cc.exe || -die "Could not make sure that MSys cc is found instead of MinGW one" cd msys/rt && -release=MSYS-g$(git show -s --pretty=%h HEAD) && -(export MSYSTEM=MSYS && - export PATH=/bin:$PATH && - (test -d bld || mkdir bld) && - cd bld && - DLL=i686-pc-msys/winsup/cygwin/new-msys-1.0.dll && - (test -f Makefile && test -z "$debug_clean" || - ../src/configure --prefix=/usr) && - (test -z "$debug" || perl -i.bak -pe 's/-O2//g' $(find -name Makefile)) && - (test -z "$debug_clean" || make clean) && - (make || test -f $DLL) && - (test ! -z "$debug" || strip $DLL) && - rebase -b 0x68000000 $DLL && - mv $DLL /bin/) && -cd / && -hash=$(git hash-object -w bin/new-msys-1.0.dll) && -git update-index --cacheinfo 100755 $hash bin/msys-1.0.dll && -git commit -s -m "Updated msys-1.0.dll to $release" && -/share/msysGit/post-checkout-hook HEAD^ HEAD 1 +release=MSYS-g$(git show -s --pretty=%h HEAD) || +die "Could not detect MSYS release" + + +# build msys.dll in native win32/msys environment +release_native() { + test -f /bin/cc.exe || ln gcc.exe /bin/cc.exe || + die "Could not make sure that MSys cc is found instead of MinGW one" + + (export MSYSTEM=MSYS && + export PATH=/bin:$PATH && + (test -d bld || mkdir bld) && + cd bld && + DLL=i686-pc-msys/winsup/cygwin/new-msys-1.0.dll && + (test -f Makefile && test -z "$debug_clean" || + ../src/configure --prefix=/usr) && + (test -z "$debug" || perl -i.bak -pe 's/-O2//g' $(find -name Makefile)) && + (test -z "$debug_clean" || make clean) && + (make || test -f $DLL) && + (test ! -z "$debug" || strip $DLL) && + rebase -b 0x68000000 $DLL && + mv $DLL /bin/) && + cd / && + hash=$(git hash-object -w bin/new-msys-1.0.dll) && + git update-index --cacheinfo 100755 $hash bin/msys-1.0.dll && + git commit -s -m "Updated msys-1.0.dll to $release" && + /share/msysGit/post-checkout-hook HEAD^ HEAD 1 +} + +# cross-build msys.dll +# XXX $debug not handled +# XXX $debug_clean not handled +release_cross() { + root=$(cd ../../../../ && pwd) && + w32api=$root/mingw/include && + ( + (test -d bld || mkdir bld) && + cd bld + + # this is the cross prefix for mingw gcc on Debian + X=i586-mingw32msvc + + DLL=$X/winsup/cygwin/new-msys-1.0.dll + + export CC=$X-gcc + export CXX=$X-g++ + export AR=$X-ar + export AS=$X-as + export RANLIB=$X-ranlib + export LD=$X-ld + export DLLTOOL=$X-dlltool + export WINDRES=$X-windres + export STRIP=$X-strip + + ( + test -d $X || mkdir $X || die + cd $X + + # strip0d config.sub in srctree + # Unfortunately I have to do this in scrtree, yes :( or else on linux configure says: + # + # configure: error: can not run + # .../config.sub + # + # and manually inspecting config.sub gives: + # + # $ ./config.sub + # bash: ./config.sub: /bin/sh^M: bad interpreter: No such file or directory + # + # this is better done in a patch, but git has problems applying + # patches with trailing ^M, and also it would be fragile to apply + # such patch from under msys. Another possibility would be to somehow + # fool configure to take config.sub from objtree, and yes, this can + # be done for newlib, but not for winsup/cygwin - the latter looks + # for aux files only in srctree. + # + # So be it - we'll dirty the tree a bit, but this is relatively + # harmles, since noone of us usually touches config.sub, and also + # even after stripping, it runs ok under msys. + ac_config_sub=../../src/config.sub + tmpfile=`mktemp -t strip0d.XXXXXX` && + tr -d '\015' <$ac_config_sub >$tmpfile && + mv $tmpfile $ac_config_sub && + chmod +x $ac_config_sub || + die "Could not tweak config.sub" + + # shadow for needed libiberty bits + test -d libiberty || + (echo ">>> shadow libiberty ..." && + mkdir libiberty && + ln -s -t libiberty/ ../../../src/libiberty/{random,strsignal}.c && + ln -s -t libiberty/ ../../../src/include/{ansidecl,libiberty}.h + ) || + die "Could not shadow libiberty" + + test -f newlib/Makefile || + (echo -e ">>> configure newlib ...\n" && + mkdir -p newlib && + cd newlib && + + # NOTE i686-pc-cygwin here! + CPPFLAGS="-D__MSYS__" \ + ../../../src/newlib/configure \ + --host=i686-pc-cygwin + ) || + die "Could not cross-configure newlib" + + test -f winsup/cygwin/Makefile || + (echo -e "\n\n>>> configure winsup/cygwin ...\n" + mkdir -p winsup/cygwin && + cd winsup/cygwin && + + # NOTE CFLAGS="-gstabs+ ..." is incompatible with wine loader + # -isystem mingw/include for w32api *.h files + CFLAGS="-O2 -D__MSYS__ -isystem $w32api" \ + ../../../../src/winsup/cygwin/configure \ + --host=$X \ + ) || + die "Could not cross-configure winsup/cygwin" + + make -C newlib || die "Could not build newlib" + make -C winsup/cygwin || die "Could not build msys.dll" + ) && + + # TODO rebase $DLL (?) + + mv $DLL $root/bin/msys-1.0.dll && + cd $root && + git commit -s -m "Updated msys-1.0.dll to cross-compiled $release" bin/msys-1.0.dll + ) +} + + +if test -z "$cross" ; then + release_native +else + release_cross +fi ^ permalink raw reply related [flat|nested] 46+ messages in thread
* cross-compiling msys-1.0.dll, was Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 19:07 ` Kirill Smelkov @ 2011-02-26 19:58 ` Johannes Schindelin 2011-03-12 13:18 ` Kirill Smelkov 0 siblings, 1 reply; 46+ messages in thread From: Johannes Schindelin @ 2011-02-26 19:58 UTC (permalink / raw) To: Kirill Smelkov Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit Hi, On Sat, 26 Feb 2011, Kirill Smelkov wrote: > On Sat, Feb 26, 2011 at 02:07:40PM +0300, Kirill Smelkov wrote: > > > On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > > > > > And lastly, in [2] you claim that you cross-built msys-1.0.dll. > > > > > I would like to have a script doing that in msysgit.git. > > > > > > > > This is in progress. Preliminary stuff (cross-built msys.dll no > > > > longer needs mingwm10.dll), is here: > > > > > > > > http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr > > > > http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys > > > > > > Thanks, I will try to find some time to test this next week. > > > > Please don't - it does not build out of the box from msysgit yet. I > > just wanted to show it is not staying stale. When it is finished, I'll > > let you know. > > Done. Please do > > git pull git://repo.or.cz/msysgit/kirr.git ks/crossmsys # into msys This is beautiful work. I will keep this as my candy tomorrow, because I still have to finish something else (after which I want to provide myself with some reward). Ciao, Dscho ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: cross-compiling msys-1.0.dll, was Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 19:58 ` cross-compiling msys-1.0.dll, was " Johannes Schindelin @ 2011-03-12 13:18 ` Kirill Smelkov 2011-03-12 15:38 ` Johannes Schindelin 0 siblings, 1 reply; 46+ messages in thread From: Kirill Smelkov @ 2011-03-12 13:18 UTC (permalink / raw) To: Johannes Schindelin Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit On Sat, Feb 26, 2011 at 08:58:16PM +0100, Johannes Schindelin wrote: > Hi, > > On Sat, 26 Feb 2011, Kirill Smelkov wrote: > > > On Sat, Feb 26, 2011 at 02:07:40PM +0300, Kirill Smelkov wrote: > > > > > On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > > > > > > And lastly, in [2] you claim that you cross-built msys-1.0.dll. > > > > > > I would like to have a script doing that in msysgit.git. > > > > > > > > > > This is in progress. Preliminary stuff (cross-built msys.dll no > > > > > longer needs mingwm10.dll), is here: > > > > > > > > > > http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr > > > > > http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys > > > > > > > > Thanks, I will try to find some time to test this next week. > > > > > > Please don't - it does not build out of the box from msysgit yet. I > > > just wanted to show it is not staying stale. When it is finished, I'll > > > let you know. > > > > Done. Please do > > > > git pull git://repo.or.cz/msysgit/kirr.git ks/crossmsys # into msys > > This is beautiful work. I will keep this as my candy tomorrow, because I > still have to finish something else (after which I want to provide myself > with some reward). So how was it, tasteful? :) Thanks, Kirill ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: cross-compiling msys-1.0.dll, was Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-03-12 13:18 ` Kirill Smelkov @ 2011-03-12 15:38 ` Johannes Schindelin 0 siblings, 0 replies; 46+ messages in thread From: Johannes Schindelin @ 2011-03-12 15:38 UTC (permalink / raw) To: Kirill Smelkov Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit Hi, On Sat, 12 Mar 2011, Kirill Smelkov wrote: > On Sat, Feb 26, 2011 at 08:58:16PM +0100, Johannes Schindelin wrote: > > Hi, > > > > On Sat, 26 Feb 2011, Kirill Smelkov wrote: > > > > > On Sat, Feb 26, 2011 at 02:07:40PM +0300, Kirill Smelkov wrote: > > > > > > > On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > > > > > > > And lastly, in [2] you claim that you cross-built msys-1.0.dll. > > > > > > > I would like to have a script doing that in msysgit.git. > > > > > > > > > > > > This is in progress. Preliminary stuff (cross-built msys.dll no > > > > > > longer needs mingwm10.dll), is here: > > > > > > > > > > > > http://repo.or.cz/w/msys/kirr.git/shortlog/refs/heads/x/kirr > > > > > > http://repo.or.cz/w/msysgit/kirr.git/shortlog/refs/heads/ks/crossmsys > > > > > > > > > > Thanks, I will try to find some time to test this next week. > > > > > > > > Please don't - it does not build out of the box from msysgit yet. I > > > > just wanted to show it is not staying stale. When it is finished, I'll > > > > let you know. > > > > > > Done. Please do > > > > > > git pull git://repo.or.cz/msysgit/kirr.git ks/crossmsys # into msys > > > > This is beautiful work. I will keep this as my candy tomorrow, because I > > still have to finish something else (after which I want to provide myself > > with some reward). > > So how was it, tasteful? :) Yep, but I got interrupted, and did not have time to come back yet. Please bear with me for a few more days! Ciao, Dscho ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 11:07 ` Kirill Smelkov 2011-02-26 19:07 ` Kirill Smelkov @ 2011-02-27 14:28 ` Johannes Schindelin 2011-03-12 13:16 ` Kirill Smelkov 1 sibling, 1 reply; 46+ messages in thread From: Johannes Schindelin @ 2011-02-27 14:28 UTC (permalink / raw) To: Kirill Smelkov Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit Hi Kirill, On Sat, 26 Feb 2011, Kirill Smelkov wrote: > On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > > > > On Fri, 25 Feb 2011, Kirill Smelkov wrote: > > > > > On Fri, Feb 25, 2011 at 12:11:59PM +0100, Johannes Schindelin wrote: > > > > > > > Further, I think that my beloved Shift+Insert will no longer work > > > > with your [2]. > > > > > > Probably yes, > > > > In my experiment after rebuilding msys-1.0.dll, it still works. > > xser32.dll has nothing to do with msys - it's just a fake stub for > sh.exe. Before testing, have you "patched" sh.exe the way it is done in > my ks/xser32.dll > (http://repo.or.cz/w/msysgit/kirr.git/commit/9d952c74a52f577b2d16d4e4a489541a8fa7fbbd) Ah. I had to add -Wl,-kill-at to the LDFLAGS so it works. See the ks/xser32 branch (and the ks/msys branch) of msysgit.git. > > The problem for now is that when I time /share/msysGit/run-tests.sh, > > there is hardly any gain from your patches: > > > > Old: > > > > > > real 18m1.031s > > user 6m17.861s > > sys 19m25.257s > > > > New: > > > > real 17m54.500s > > user 6m12.319s > > sys 19m28.567s > > Did you patch sh.exe to link to xser32.dll instead of user32.dll? Now I did, and the difference is quite noticable: real 15m37.281s user 5m6.934s sys 15m53.911s (Note that I did not spend time to increase the N, so there is no point in putting a percentage on the difference.) I wonder whether we could patch sh.exe so it loads user32.dll _lazily_ rather than using the xser32.dll hack? (This would also fix the issue that I can no longer use Shift+Insert to paste the clipboard into the Git Bash...) > Also I can't say for sure (hope yet) how sh-intensitive git tests are, > but at least running configure for say gettext or whatever should be > visibly faster, at least on wine. They are pretty intensive, as they are shell scripts all over the place in their own right. I fear, however, that the real problem (maybe not on WINE, but with real Windows) is that sh.exe needs to provide too many POSIX-like things when starting new processes. I fear, further, that the only way to make things more efficient on Windows is to get rid of the many exec() calls and do more things in-process. (If this is true, JGit should kick msysGit's ass.) This would not apply to the test suite, though, which needs to stay a shell script suite. But maybe I am wrong, and the performance is lost through memory management and filesystem interaction. Ciao, Dscho ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-27 14:28 ` Johannes Schindelin @ 2011-03-12 13:16 ` Kirill Smelkov 0 siblings, 0 replies; 46+ messages in thread From: Kirill Smelkov @ 2011-03-12 13:16 UTC (permalink / raw) To: Johannes Schindelin Cc: Erik Faye-Lund, Ævar Arnfjörð Bjarmason, Junio C Hamano, git, msysGit [ Sorry for silence, completly buried under load... ] On Sun, Feb 27, 2011 at 03:28:43PM +0100, Johannes Schindelin wrote: > Hi Kirill, > > On Sat, 26 Feb 2011, Kirill Smelkov wrote: > > > On Fri, Feb 25, 2011 at 10:54:40PM +0100, Johannes Schindelin wrote: > > > > > > On Fri, 25 Feb 2011, Kirill Smelkov wrote: > > > > > > > On Fri, Feb 25, 2011 at 12:11:59PM +0100, Johannes Schindelin wrote: > > > > > > > > > Further, I think that my beloved Shift+Insert will no longer work > > > > > with your [2]. > > > > > > > > Probably yes, > > > > > > In my experiment after rebuilding msys-1.0.dll, it still works. > > > > xser32.dll has nothing to do with msys - it's just a fake stub for > > sh.exe. Before testing, have you "patched" sh.exe the way it is done in > > my ks/xser32.dll > > (http://repo.or.cz/w/msysgit/kirr.git/commit/9d952c74a52f577b2d16d4e4a489541a8fa7fbbd) > > Ah. I had to add -Wl,-kill-at to the LDFLAGS so it works. > > See the ks/xser32 branch (and the ks/msys branch) of msysgit.git. Hm, strange how it used to work on my side :) but you are right, thanks. > > > > The problem for now is that when I time /share/msysGit/run-tests.sh, > > > there is hardly any gain from your patches: > > > > > > Old: > > > > > > > > > real 18m1.031s > > > user 6m17.861s > > > sys 19m25.257s > > > > > > New: > > > > > > real 17m54.500s > > > user 6m12.319s > > > sys 19m28.567s > > > > Did you patch sh.exe to link to xser32.dll instead of user32.dll? > > Now I did, and the difference is quite noticable: > > real 15m37.281s > user 5m6.934s > sys 15m53.911s This is on real windows, yes? > (Note that I did not spend time to increase the N, so there is no point in > putting a percentage on the difference.) > > I wonder whether we could patch sh.exe so it loads user32.dll _lazily_ > rather than using the xser32.dll hack? > > (This would also fix the issue that I can no longer use Shift+Insert to > paste the clipboard into the Git Bash...) The patch for bash would be simple enough to re-link it with user32.dll `dlltool -y` 'ed (-y creates delay-load import lobrary), or simply to patch lib/readline/kill.c to delay-load clipboard functions manually here: /* A special paste command for users of Cygnus's cygwin32. */ #if defined (__CYGWIN__) #include <windows.h> int rl_paste_from_clipboard (count, key) int count, key; { char *data, *ptr; int len; if (OpenClipboard (NULL) == 0) return (0); data = (char *)GetClipboardData (CF_TEXT); if (data) ... Now this is simple only after we have infrastructure for rebuilding bash in place, and untill we don't, something like this could help in the meantime (sorry, can't test it properly under wine): ---- 8< ---- From 92151fe8325aa8c39e1b77b9780ed562703ba008 Mon Sep 17 00:00:00 2001 From: Kirill Smelkov <kirr@mns.spb.ru> Date: Sat, 12 Mar 2011 15:20:19 +0300 Subject: [PATCH] xser32: be a delay-load proxy to real user32.dll This way we'll hopefully make clipboard work in shell again. The proper way is of course to rebuild sh.exe with user32 being `dlltool -y` 'ed. Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> --- src/xser32/Makefile | 3 +- src/xser32/release.sh | 2 +- src/xser32/xser32.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/xser32/Makefile b/src/xser32/Makefile index 818ab49..f201768 100644 --- a/src/xser32/Makefile +++ b/src/xser32/Makefile @@ -2,9 +2,10 @@ CC = gcc CFLAGS = -Wall -Wwrite-strings LD = gcc LDFLAGS = -nostdlib -Wl,-kill-at +LIBS = -lkernel32 xser32.dll: xser32.o - $(LD) $(LDFLAGS) -shared -o $@ $^ -e _DllMain@12 + $(LD) $(LDFLAGS) -shared -o $@ $^ -e _DllMain@12 $(LIBS) clean: -rm -f xser32.o xser32.dll diff --git a/src/xser32/release.sh b/src/xser32/release.sh index 981f5e8..64e31b4 100644 --- a/src/xser32/release.sh +++ b/src/xser32/release.sh @@ -5,4 +5,4 @@ cd "$(dirname "$0")" && make && install -m 775 xser32.dll $DEST && git add $DEST && -git commit -s -m "xser32.dll: Fake user32.dll like stub (for sh.exe not to load user32.dll)" $DEST +git commit -s -m "xser32.dll: Delay-load proxy to real user32.dll (for sh.exe not to load user32.dll on every call)" $DEST diff --git a/src/xser32/xser32.c b/src/xser32/xser32.c index 27375ab..362a80b 100644 --- a/src/xser32/xser32.c +++ b/src/xser32/xser32.c @@ -1,22 +1,77 @@ /* fake clipboard routines for sh.exe not to load user32.dll */ #include <windef.h> +#include <stdarg.h> +#include <winbase.h> + +static HMODULE user32; + +static void die(const char *msg) +{ + const char *p; + DWORD msglen; + DWORD written; + + /* hand-made strlen */ + for (p=msg, msglen=0; *p; ++p, ++msglen) + ; + + WriteFile (GetStdHandle (STD_ERROR_HANDLE), msg, msglen, &written, 0); + ExitProcess(1); +} + +static void import_user32() +{ + if (user32) + return; + + user32 = LoadLibrary("user32.dll"); + if (!user32) + die("E: cannot load user32.dll\n"); +} __declspec(dllexport) BOOL WINAPI OpenClipboard(HWND hWndNewOwner) { - return FALSE; /* always fails */ + static BOOL WINAPI (*pOpenClipboard) (HWND); + + if (!pOpenClipboard) { + import_user32(); + pOpenClipboard = GetProcAddress(user32, "OpenClipboard"); + if (!pOpenClipboard) + die("E: cannot import OpenClipboard from user32.dll\n"); + } + + return pOpenClipboard(hWndNewOwner); } __declspec(dllexport) BOOL WINAPI CloseClipboard(void) { - return TRUE; /* ok */ + static BOOL WINAPI (*pCloseClipboard) (void); + + if (!pCloseClipboard) { + import_user32(); + pCloseClipboard = GetProcAddress(user32, "CloseClipboard"); + if (!pCloseClipboard) + die("E: cannot import CloseClipboard from user32.dll\n"); + } + + return pCloseClipboard(); } __declspec(dllexport) HANDLE WINAPI GetClipboardData(UINT uFormat) { - return NULL; /* always fail */ + static HANDLE WINAPI (*pGetClipboardData) (UINT); + + if (!pGetClipboardData) { + import_user32(); + pGetClipboardData = GetProcAddress(user32, "GetClipboardData"); + if (!pGetClipboardData) + die("E: cannot import GetClipboardData from user32.dll\n"); + } + + return GetClipboardData(uFormat); } -- 1.7.4.1.225.g83c3c ---- 8< ---- > > > Also I can't say for sure (hope yet) how sh-intensitive git tests are, > > but at least running configure for say gettext or whatever should be > > visibly faster, at least on wine. > > They are pretty intensive, as they are shell scripts all over the place in > their own right. Yes, thanks, your timing justifies that. > I fear, however, that the real problem (maybe not on > WINE, but with real Windows) is that sh.exe needs to provide too many > POSIX-like things when starting new processes. > > I fear, further, that the only way to make things more efficient on > Windows is to get rid of the many exec() calls and do more things > in-process. (If this is true, JGit should kick msysGit's ass.) This would > not apply to the test suite, though, which needs to stay a shell script > suite. > > But maybe I am wrong, and the performance is lost through memory > management and filesystem interaction. I'd say we should not fear and we should not guess. The user32.dll stripping was low-hanging-fruit, and judging from it I'd say there should be other such places where time is wasted because nobody cared yet. Only could we please move forward, and try to integrate somehow the result to devel? Or else I fear we'll have too many half-done work/* branches... > Ciao, > Dscho Thanks, Kirill ^ permalink raw reply related [flat|nested] 46+ messages in thread
* Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-23 23:26 What's cooking in git.git (Feb 2011, #05; Wed, 23) Junio C Hamano ` (2 preceding siblings ...) 2011-02-24 0:32 ` Ævar Arnfjörð Bjarmason @ 2011-02-26 10:24 ` Sebastian Schuberth 2011-02-26 10:33 ` Junio C Hamano 2011-02-27 5:38 ` Chris Packham 3 siblings, 2 replies; 46+ messages in thread From: Sebastian Schuberth @ 2011-02-26 10:24 UTC (permalink / raw) To: judge.packham; +Cc: Junio C Hamano, git, charles [-- Attachment #1: Type: text/plain, Size: 1823 bytes --] On 24.02.2011 00:26, Junio C Hamano wrote: > * cp/mergetool-beyondcompare (2011-02-18) 1 commit > - mergetool--lib: add support for beyond compare Sorry for not responding earlier to this, but problems at my news provider seem to have swallowed mails from several days, including the original post of http://marc.info/?l=git&m=129801656713478&w=2 A while ago, I had already proposed http://marc.info/?l=git&m=129007741814521&w=2 I'm not entirely sure why it was ignored in the end, probably I did not report back to have tested it in Linux. A few things that I like better in my patch than in Chris': - Beyond Compare is added as "bc3" instead of "bcompare", which is both shorter and indicates that only version 3, not version 2, is supported. - Chris seems to be missing the patch to git-gui/lib/mergetool.tcl - To the best of my knownledge, the Beyond Compare executable is called "BCompare" (note the case), that means even with the merge tool named "bcompare" a translation step in git-mergetool--lib.sh should by required (as done in my patch). Chris, as you seem to have tested ion Linux, could you shed a light on this? - Using dashes for the options to Beyond Compare is fine on Windows, however, I believe the order of the files is wrong, although that might be a bit subjective. For a 3-way merge the syntax is BCompare.exe C:\Left.ext C:\Right.ext C:\Center.ext So the file that should go to the center panel is specified last. AFAIK, all other merge tools are called such that $BASE goes to the center. This is why my patch specifies $BASE last. Any more opinions? Chris, in case you'd agree to prefer my patch, I'd be very grateful if you could test it on Linux. For your convenience, I've rebased onto the current master and attached the patch files. -- Sebastian Schuberth [-- Attachment #2: 0001-mergetool-lib-Sort-tools-alphabetically-for-easier-l.patch --] [-- Type: text/plain, Size: 16845 bytes --] From 9016c0c0fbacba2747fc0023c853edd6a3d0da87 Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth <sschuberth@gmail.com> Date: Thu, 18 Nov 2010 10:23:51 +0100 Subject: [PATCH 1/3] mergetool--lib: Sort tools alphabetically for easier lookup Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com> --- Documentation/git-difftool.txt | 4 +- Documentation/git-mergetool.txt | 4 +- Documentation/merge-config.txt | 8 +- git-gui/lib/mergetool.tcl | 94 ++++++++-------- git-mergetool--lib.sh | 245 +++++++++++++++++++-------------------- 5 files changed, 177 insertions(+), 178 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index db87f1d..4c8825d 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -31,8 +31,8 @@ OPTIONS --tool=<tool>:: Use the diff tool specified by <tool>. Valid merge tools are: - kdiff3, kompare, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, - ecmerge, diffuse, opendiff, p4merge and araxis. + araxis, diffuse, emerge, ecmerge, gvimdiff, kdiff3, + kompare, meld, opendiff, p4merge, tkdiff, vimdiff and xxdiff. + If a diff tool is not specified, 'git difftool' will use the configuration variable `diff.tool`. If the diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 1f75a84..4987245 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -26,8 +26,8 @@ OPTIONS --tool=<tool>:: Use the merge resolution program specified by <tool>. Valid merge tools are: - kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge, - diffuse, tortoisemerge, opendiff, p4merge and araxis. + araxis, diffuse, ecmerge, emerge, gvimdiff, kdiff3, + meld, opendiff, p4merge, tkdiff, tortoisemerge, vimdiff and xxdiff. + If a merge resolution program is not specified, 'git mergetool' will use the configuration variable `merge.tool`. If the diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 1e5c22c..90587db 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -33,10 +33,10 @@ merge.stat:: merge.tool:: Controls which merge resolution program is used by - linkgit:git-mergetool[1]. Valid built-in values are: "kdiff3", - "tkdiff", "meld", "xxdiff", "emerge", "vimdiff", "gvimdiff", - "diffuse", "ecmerge", "tortoisemerge", "p4merge", "araxis" and - "opendiff". Any other value is treated is custom merge tool + linkgit:git-mergetool[1]. Valid built-in values are: "araxis", + "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld", + "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff" + and "xxdiff". Any other value is treated is custom merge tool and there must be a corresponding mergetool.<tool>.cmd option. merge.verbosity:: diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl index 3fe90e6..249e0cf 100644 --- a/git-gui/lib/mergetool.tcl +++ b/git-gui/lib/mergetool.tcl @@ -175,43 +175,49 @@ proc merge_resolve_tool2 {} { # Build the command line switch -- $tool { - kdiff3 { + araxis { if {$base_stage ne {}} { - set cmdline [list "$merge_tool_path" --auto --L1 "$MERGED (Base)" \ - --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE"] + set cmdline [list "$merge_tool_path" -wait -merge -3 -a1 \ + -title1:"'$MERGED (Base)'" -title2:"'$MERGED (Local)'" \ + -title3:"'$MERGED (Remote)'" \ + "$BASE" "$LOCAL" "$REMOTE" "$MERGED"] } else { - set cmdline [list "$merge_tool_path" --auto --L1 "$MERGED (Local)" \ - --L2 "$MERGED (Remote)" -o "$MERGED" "$LOCAL" "$REMOTE"] + set cmdline [list "$merge_tool_path" -wait -2 \ + -title1:"'$MERGED (Local)'" -title2:"'$MERGED (Remote)'" \ + "$LOCAL" "$REMOTE" "$MERGED"] } } - tkdiff { + ecmerge { if {$base_stage ne {}} { - set cmdline [list "$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"] + set cmdline [list "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"] } else { - set cmdline [list "$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"] + set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"] } } - meld { - set cmdline [list "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"] + emerge { + if {$base_stage ne {}} { + set cmdline [list "$merge_tool_path" -f emerge-files-with-ancestor-command \ + "$LOCAL" "$REMOTE" "$BASE" "$basename"] + } else { + set cmdline [list "$merge_tool_path" -f emerge-files-command \ + "$LOCAL" "$REMOTE" "$basename"] + } } gvimdiff { set cmdline [list "$merge_tool_path" -f "$LOCAL" "$MERGED" "$REMOTE"] } - xxdiff { + kdiff3 { if {$base_stage ne {}} { - set cmdline [list "$merge_tool_path" -X --show-merged-pane \ - -R {Accel.SaveAsMerged: "Ctrl-S"} \ - -R {Accel.Search: "Ctrl+F"} \ - -R {Accel.SearchForward: "Ctrl-G"} \ - --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"] + set cmdline [list "$merge_tool_path" --auto --L1 "$MERGED (Base)" \ + --L2 "$MERGED (Local)" --L3 "$MERGED (Remote)" -o "$MERGED" "$BASE" "$LOCAL" "$REMOTE"] } else { - set cmdline [list "$merge_tool_path" -X --show-merged-pane \ - -R {Accel.SaveAsMerged: "Ctrl-S"} \ - -R {Accel.Search: "Ctrl+F"} \ - -R {Accel.SearchForward: "Ctrl-G"} \ - --merged-file "$MERGED" "$LOCAL" "$REMOTE"] + set cmdline [list "$merge_tool_path" --auto --L1 "$MERGED (Local)" \ + --L2 "$MERGED (Remote)" -o "$MERGED" "$LOCAL" "$REMOTE"] } } + meld { + set cmdline [list "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"] + } opendiff { if {$base_stage ne {}} { set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" -ancestor "$BASE" -merge "$MERGED"] @@ -219,22 +225,20 @@ proc merge_resolve_tool2 {} { set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" -merge "$MERGED"] } } - ecmerge { - if {$base_stage ne {}} { - set cmdline [list "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"] - } else { - set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" --default --mode=merge2 --to="$MERGED"] - } + p4merge { + set cmdline [list "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"] } - emerge { + tkdiff { if {$base_stage ne {}} { - set cmdline [list "$merge_tool_path" -f emerge-files-with-ancestor-command \ - "$LOCAL" "$REMOTE" "$BASE" "$basename"] + set cmdline [list "$merge_tool_path" -a "$BASE" -o "$MERGED" "$LOCAL" "$REMOTE"] } else { - set cmdline [list "$merge_tool_path" -f emerge-files-command \ - "$LOCAL" "$REMOTE" "$basename"] + set cmdline [list "$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"] } } + vimdiff { + error_popup [mc "Not a GUI merge tool: '%s'" $tool] + return + } winmerge { if {$base_stage ne {}} { # This tool does not support 3-way merges. @@ -245,25 +249,21 @@ proc merge_resolve_tool2 {} { -dl "Theirs File" -dr "Mine File" "$REMOTE" "$LOCAL" "$MERGED"] } } - araxis { + xxdiff { if {$base_stage ne {}} { - set cmdline [list "$merge_tool_path" -wait -merge -3 -a1 \ - -title1:"'$MERGED (Base)'" -title2:"'$MERGED (Local)'" \ - -title3:"'$MERGED (Remote)'" \ - "$BASE" "$LOCAL" "$REMOTE" "$MERGED"] + set cmdline [list "$merge_tool_path" -X --show-merged-pane \ + -R {Accel.SaveAsMerged: "Ctrl-S"} \ + -R {Accel.Search: "Ctrl+F"} \ + -R {Accel.SearchForward: "Ctrl-G"} \ + --merged-file "$MERGED" "$LOCAL" "$BASE" "$REMOTE"] } else { - set cmdline [list "$merge_tool_path" -wait -2 \ - -title1:"'$MERGED (Local)'" -title2:"'$MERGED (Remote)'" \ - "$LOCAL" "$REMOTE" "$MERGED"] + set cmdline [list "$merge_tool_path" -X --show-merged-pane \ + -R {Accel.SaveAsMerged: "Ctrl-S"} \ + -R {Accel.Search: "Ctrl+F"} \ + -R {Accel.SearchForward: "Ctrl-G"} \ + --merged-file "$MERGED" "$LOCAL" "$REMOTE"] } } - p4merge { - set cmdline [list "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"] - } - vimdiff { - error_popup [mc "Not a GUI merge tool: '%s'" $tool] - return - } default { error_popup [mc "Unsupported merge tool '%s'" $tool] return diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 77d4aee..9fb82e5 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -10,17 +10,17 @@ merge_mode() { translate_merge_tool_path () { case "$1" in - vimdiff|vimdiff2) - echo vim - ;; - gvimdiff|gvimdiff2) - echo gvim + araxis) + echo compare ;; emerge) echo emacs ;; - araxis) - echo compare + gvimdiff|gvimdiff2) + echo gvim + ;; + vimdiff|vimdiff2) + echo vim ;; *) echo "$1" @@ -46,17 +46,16 @@ check_unchanged () { valid_tool () { case "$1" in - kdiff3 | tkdiff | xxdiff | meld | opendiff | \ - vimdiff | gvimdiff | vimdiff2 | gvimdiff2 | \ - emerge | ecmerge | diffuse | araxis | p4merge) + araxis | diffuse | ecmerge | emerge | gvimdiff | gvimdiff2 | \ + kdiff3 | meld | opendiff | p4merge | tkdiff | vimdiff | vimdiff2 | xxdiff) ;; # happy - tortoisemerge) - if ! merge_mode; then + kompare) + if ! diff_mode; then return 1 fi ;; - kompare) - if ! diff_mode; then + tortoisemerge) + if ! merge_mode; then return 1 fi ;; @@ -89,69 +88,22 @@ run_merge_tool () { status=0 case "$1" in - kdiff3) - if merge_mode; then - if $base_present; then - ("$merge_tool_path" --auto \ - --L1 "$MERGED (Base)" \ - --L2 "$MERGED (Local)" \ - --L3 "$MERGED (Remote)" \ - -o "$MERGED" \ - "$BASE" "$LOCAL" "$REMOTE" \ - > /dev/null 2>&1) - else - ("$merge_tool_path" --auto \ - --L1 "$MERGED (Local)" \ - --L2 "$MERGED (Remote)" \ - -o "$MERGED" \ - "$LOCAL" "$REMOTE" \ - > /dev/null 2>&1) - fi - status=$? - else - ("$merge_tool_path" --auto \ - --L1 "$MERGED (A)" \ - --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \ - > /dev/null 2>&1) - fi - ;; - kompare) - "$merge_tool_path" "$LOCAL" "$REMOTE" - ;; - tkdiff) - if merge_mode; then - if $base_present; then - "$merge_tool_path" -a "$BASE" \ - -o "$MERGED" "$LOCAL" "$REMOTE" - else - "$merge_tool_path" \ - -o "$MERGED" "$LOCAL" "$REMOTE" - fi - status=$? - else - "$merge_tool_path" "$LOCAL" "$REMOTE" - fi - ;; - p4merge) + araxis) if merge_mode; then - touch "$BACKUP" + touch "$BACKUP" if $base_present; then - "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED" + "$merge_tool_path" -wait -merge -3 -a1 \ + "$BASE" "$LOCAL" "$REMOTE" "$MERGED" \ + >/dev/null 2>&1 else - "$merge_tool_path" "$LOCAL" "$LOCAL" "$REMOTE" "$MERGED" + "$merge_tool_path" -wait -2 \ + "$LOCAL" "$REMOTE" "$MERGED" \ + >/dev/null 2>&1 fi check_unchanged else - "$merge_tool_path" "$LOCAL" "$REMOTE" - fi - ;; - meld) - if merge_mode; then - touch "$BACKUP" - "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" - check_unchanged - else - "$merge_tool_path" "$LOCAL" "$REMOTE" + "$merge_tool_path" -wait -2 "$LOCAL" "$REMOTE" \ + >/dev/null 2>&1 fi ;; diffuse) @@ -170,23 +122,58 @@ run_merge_tool () { "$merge_tool_path" "$LOCAL" "$REMOTE" | cat fi ;; - vimdiff|gvimdiff) + ecmerge) if merge_mode; then touch "$BACKUP" if $base_present; then - "$merge_tool_path" -f -d -c "wincmd J" \ - "$MERGED" "$LOCAL" "$BASE" "$REMOTE" + "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \ + --default --mode=merge3 --to="$MERGED" else - "$merge_tool_path" -f -d -c "wincmd l" \ - "$LOCAL" "$MERGED" "$REMOTE" + "$merge_tool_path" "$LOCAL" "$REMOTE" \ + --default --mode=merge2 --to="$MERGED" fi check_unchanged else - "$merge_tool_path" -f -d -c "wincmd l" \ + "$merge_tool_path" --default --mode=diff2 \ "$LOCAL" "$REMOTE" fi ;; - vimdiff2|gvimdiff2) + emerge) + if merge_mode; then + if $base_present; then + "$merge_tool_path" \ + -f emerge-files-with-ancestor-command \ + "$LOCAL" "$REMOTE" "$BASE" \ + "$(basename "$MERGED")" + else + "$merge_tool_path" \ + -f emerge-files-command \ + "$LOCAL" "$REMOTE" \ + "$(basename "$MERGED")" + fi + status=$? + else + "$merge_tool_path" -f emerge-files-command \ + "$LOCAL" "$REMOTE" + fi + ;; + gvimdiff|vimdiff) + if merge_mode; then + touch "$BACKUP" + if $base_present; then + "$merge_tool_path" -f -d -c "wincmd J" \ + "$MERGED" "$LOCAL" "$BASE" "$REMOTE" + else + "$merge_tool_path" -f -d -c "wincmd l" \ + "$LOCAL" "$MERGED" "$REMOTE" + fi + check_unchanged + else + "$merge_tool_path" -f -d -c "wincmd l" \ + "$LOCAL" "$REMOTE" + fi + ;; + gvimdiff2|vimdiff2) if merge_mode; then touch "$BACKUP" "$merge_tool_path" -f -d -c "wincmd l" \ @@ -197,30 +184,42 @@ run_merge_tool () { "$LOCAL" "$REMOTE" fi ;; - xxdiff) + kdiff3) if merge_mode; then - touch "$BACKUP" if $base_present; then - "$merge_tool_path" -X --show-merged-pane \ - -R 'Accel.SaveAsMerged: "Ctrl-S"' \ - -R 'Accel.Search: "Ctrl+F"' \ - -R 'Accel.SearchForward: "Ctrl-G"' \ - --merged-file "$MERGED" \ - "$LOCAL" "$BASE" "$REMOTE" + ("$merge_tool_path" --auto \ + --L1 "$MERGED (Base)" \ + --L2 "$MERGED (Local)" \ + --L3 "$MERGED (Remote)" \ + -o "$MERGED" \ + "$BASE" "$LOCAL" "$REMOTE" \ + > /dev/null 2>&1) else - "$merge_tool_path" -X $extra \ - -R 'Accel.SaveAsMerged: "Ctrl-S"' \ - -R 'Accel.Search: "Ctrl+F"' \ - -R 'Accel.SearchForward: "Ctrl-G"' \ - --merged-file "$MERGED" \ - "$LOCAL" "$REMOTE" + ("$merge_tool_path" --auto \ + --L1 "$MERGED (Local)" \ + --L2 "$MERGED (Remote)" \ + -o "$MERGED" \ + "$LOCAL" "$REMOTE" \ + > /dev/null 2>&1) fi + status=$? + else + ("$merge_tool_path" --auto \ + --L1 "$MERGED (A)" \ + --L2 "$MERGED (B)" "$LOCAL" "$REMOTE" \ + > /dev/null 2>&1) + fi + ;; + kompare) + "$merge_tool_path" "$LOCAL" "$REMOTE" + ;; + meld) + if merge_mode; then + touch "$BACKUP" + "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" check_unchanged else - "$merge_tool_path" \ - -R 'Accel.Search: "Ctrl+F"' \ - -R 'Accel.SearchForward: "Ctrl-G"' \ - "$LOCAL" "$REMOTE" + "$merge_tool_path" "$LOCAL" "$REMOTE" fi ;; opendiff) @@ -239,39 +238,31 @@ run_merge_tool () { "$merge_tool_path" "$LOCAL" "$REMOTE" | cat fi ;; - ecmerge) + p4merge) if merge_mode; then - touch "$BACKUP" + touch "$BACKUP" if $base_present; then - "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \ - --default --mode=merge3 --to="$MERGED" + "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED" else - "$merge_tool_path" "$LOCAL" "$REMOTE" \ - --default --mode=merge2 --to="$MERGED" + "$merge_tool_path" "$LOCAL" "$LOCAL" "$REMOTE" "$MERGED" fi check_unchanged else - "$merge_tool_path" --default --mode=diff2 \ - "$LOCAL" "$REMOTE" + "$merge_tool_path" "$LOCAL" "$REMOTE" fi ;; - emerge) + tkdiff) if merge_mode; then if $base_present; then - "$merge_tool_path" \ - -f emerge-files-with-ancestor-command \ - "$LOCAL" "$REMOTE" "$BASE" \ - "$(basename "$MERGED")" + "$merge_tool_path" -a "$BASE" \ + -o "$MERGED" "$LOCAL" "$REMOTE" else "$merge_tool_path" \ - -f emerge-files-command \ - "$LOCAL" "$REMOTE" \ - "$(basename "$MERGED")" + -o "$MERGED" "$LOCAL" "$REMOTE" fi status=$? else - "$merge_tool_path" -f emerge-files-command \ - "$LOCAL" "$REMOTE" + "$merge_tool_path" "$LOCAL" "$REMOTE" fi ;; tortoisemerge) @@ -286,22 +277,30 @@ run_merge_tool () { status=1 fi ;; - araxis) + xxdiff) if merge_mode; then touch "$BACKUP" if $base_present; then - "$merge_tool_path" -wait -merge -3 -a1 \ - "$BASE" "$LOCAL" "$REMOTE" "$MERGED" \ - >/dev/null 2>&1 + "$merge_tool_path" -X --show-merged-pane \ + -R 'Accel.SaveAsMerged: "Ctrl-S"' \ + -R 'Accel.Search: "Ctrl+F"' \ + -R 'Accel.SearchForward: "Ctrl-G"' \ + --merged-file "$MERGED" \ + "$LOCAL" "$BASE" "$REMOTE" else - "$merge_tool_path" -wait -2 \ - "$LOCAL" "$REMOTE" "$MERGED" \ - >/dev/null 2>&1 + "$merge_tool_path" -X $extra \ + -R 'Accel.SaveAsMerged: "Ctrl-S"' \ + -R 'Accel.Search: "Ctrl+F"' \ + -R 'Accel.SearchForward: "Ctrl-G"' \ + --merged-file "$MERGED" \ + "$LOCAL" "$REMOTE" fi check_unchanged else - "$merge_tool_path" -wait -2 "$LOCAL" "$REMOTE" \ - >/dev/null 2>&1 + "$merge_tool_path" \ + -R 'Accel.Search: "Ctrl+F"' \ + -R 'Accel.SearchForward: "Ctrl-G"' \ + "$LOCAL" "$REMOTE" fi ;; *) -- 1.7.3.2.msysgit.6.dirty [-- Attachment #3: 0002-mergetool-lib-Add-Beyond-Compare-3-as-a-tool.patch --] [-- Type: text/plain, Size: 4866 bytes --] From ef1fa03180c78ac2f7119b0b79fddaa37195a48d Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth <sschuberth@gmail.com> Date: Thu, 18 Nov 2010 11:05:01 +0100 Subject: [PATCH 2/3] mergetool--lib: Add Beyond Compare 3 as a tool Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com> --- Documentation/git-difftool.txt | 2 +- Documentation/git-mergetool.txt | 2 +- Documentation/merge-config.txt | 2 +- contrib/completion/git-completion.bash | 2 +- git-gui/lib/mergetool.tcl | 7 +++++++ git-mergetool--lib.sh | 22 ++++++++++++++++++++-- 6 files changed, 31 insertions(+), 6 deletions(-) diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt index 4c8825d..f087eff 100644 --- a/Documentation/git-difftool.txt +++ b/Documentation/git-difftool.txt @@ -31,7 +31,7 @@ OPTIONS --tool=<tool>:: Use the diff tool specified by <tool>. Valid merge tools are: - araxis, diffuse, emerge, ecmerge, gvimdiff, kdiff3, + araxis, bc3, diffuse, emerge, ecmerge, gvimdiff, kdiff3, kompare, meld, opendiff, p4merge, tkdiff, vimdiff and xxdiff. + If a diff tool is not specified, 'git difftool' diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt index 4987245..740b3f1 100644 --- a/Documentation/git-mergetool.txt +++ b/Documentation/git-mergetool.txt @@ -26,7 +26,7 @@ OPTIONS --tool=<tool>:: Use the merge resolution program specified by <tool>. Valid merge tools are: - araxis, diffuse, ecmerge, emerge, gvimdiff, kdiff3, + araxis, bc3, diffuse, ecmerge, emerge, gvimdiff, kdiff3, meld, opendiff, p4merge, tkdiff, tortoisemerge, vimdiff and xxdiff. + If a merge resolution program is not specified, 'git mergetool' diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt index 90587db..33bf74c 100644 --- a/Documentation/merge-config.txt +++ b/Documentation/merge-config.txt @@ -34,7 +34,7 @@ merge.stat:: merge.tool:: Controls which merge resolution program is used by linkgit:git-mergetool[1]. Valid built-in values are: "araxis", - "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld", + "bc3", "diffuse", "ecmerge", "emerge", "gvimdiff", "kdiff3", "meld", "opendiff", "p4merge", "tkdiff", "tortoisemerge", "vimdiff" and "xxdiff". Any other value is treated is custom merge tool and there must be a corresponding mergetool.<tool>.cmd option. diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 893b771..058c2a9 100755 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1358,7 +1358,7 @@ _git_diff () } __git_mergetools_common="diffuse ecmerge emerge kdiff3 meld opendiff - tkdiff vimdiff gvimdiff xxdiff araxis p4merge + tkdiff vimdiff gvimdiff xxdiff araxis p4merge bc3 " _git_difftool () diff --git a/git-gui/lib/mergetool.tcl b/git-gui/lib/mergetool.tcl index 249e0cf..3c8e73b 100644 --- a/git-gui/lib/mergetool.tcl +++ b/git-gui/lib/mergetool.tcl @@ -187,6 +187,13 @@ proc merge_resolve_tool2 {} { "$LOCAL" "$REMOTE" "$MERGED"] } } + bc3 { + if {$base_stage ne {}} { + set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" -mergeoutput="$MERGED"] + } else { + set cmdline [list "$merge_tool_path" "$LOCAL" "$REMOTE" -mergeoutput="$MERGED"] + } + } ecmerge { if {$base_stage ne {}} { set cmdline [list "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" --default --mode=merge3 --to="$MERGED"] diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 9fb82e5..3ac6231 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -13,6 +13,9 @@ translate_merge_tool_path () { araxis) echo compare ;; + bc3) + echo BCompare + ;; emerge) echo emacs ;; @@ -46,7 +49,7 @@ check_unchanged () { valid_tool () { case "$1" in - araxis | diffuse | ecmerge | emerge | gvimdiff | gvimdiff2 | \ + araxis | bc3 | diffuse | ecmerge | emerge | gvimdiff | gvimdiff2 | \ kdiff3 | meld | opendiff | p4merge | tkdiff | vimdiff | vimdiff2 | xxdiff) ;; # happy kompare) @@ -106,6 +109,21 @@ run_merge_tool () { >/dev/null 2>&1 fi ;; + bc3) + if merge_mode; then + touch "$BACKUP" + if $base_present; then + "$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \ + -mergeoutput="$MERGED" + else + "$merge_tool_path" "$LOCAL" "$REMOTE" \ + -mergeoutput="$MERGED" + fi + check_unchanged + else + "$merge_tool_path" "$LOCAL" "$REMOTE" + fi + ;; diffuse) if merge_mode; then touch "$BACKUP" @@ -342,7 +360,7 @@ guess_merge_tool () { else tools="opendiff kdiff3 tkdiff xxdiff meld $tools" fi - tools="$tools gvimdiff diffuse ecmerge p4merge araxis" + tools="$tools gvimdiff diffuse ecmerge p4merge araxis bc3" fi case "${VISUAL:-$EDITOR}" in *vim*) -- 1.7.3.2.msysgit.6.dirty [-- Attachment #4: 0003-mergetool-lib-Add-the-proper-executable-name-for-ECM.patch --] [-- Type: text/plain, Size: 704 bytes --] From 8e871dab73f3759496ce5f0b87fcfca689b0c4ae Mon Sep 17 00:00:00 2001 From: Sebastian Schuberth <sschuberth@gmail.com> Date: Thu, 18 Nov 2010 11:06:13 +0100 Subject: [PATCH 3/3] mergetool--lib: Add the proper executable name for ECMerge Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com> --- git-mergetool--lib.sh | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 3ac6231..95da00b 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -16,6 +16,9 @@ translate_merge_tool_path () { bc3) echo BCompare ;; + ecmerge) + echo guimerge + ;; emerge) echo emacs ;; -- 1.7.3.2.msysgit.6.dirty ^ permalink raw reply related [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 10:24 ` Adding Beyond Compare as a merge tool, was: " Sebastian Schuberth @ 2011-02-26 10:33 ` Junio C Hamano 2011-02-26 10:48 ` Sebastian Schuberth 2011-02-27 5:38 ` Chris Packham 1 sibling, 1 reply; 46+ messages in thread From: Junio C Hamano @ 2011-02-26 10:33 UTC (permalink / raw) To: Sebastian Schuberth; +Cc: judge.packham, git, charles Sebastian Schuberth <sschuberth@gmail.com> writes: > For your convenience, I've rebased onto the current master and > attached the patch files. Could we be even more greedy (and lazy) and ask you to send them as a three-patch series, one-patch-per-message and plain-text, as well, so that these can be discussed on-list just like any other patches? ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 10:33 ` Junio C Hamano @ 2011-02-26 10:48 ` Sebastian Schuberth 0 siblings, 0 replies; 46+ messages in thread From: Sebastian Schuberth @ 2011-02-26 10:48 UTC (permalink / raw) To: Junio C Hamano; +Cc: judge.packham, git, charles On Sat, Feb 26, 2011 at 11:33, Junio C Hamano <gitster@pobox.com> wrote: >> For your convenience, I've rebased onto the current master and >> attached the patch files. > > Could we be even more greedy (and lazy) and ask you to send them as a > three-patch series, one-patch-per-message and plain-text, as well, so that > these can be discussed on-list just like any other patches? Will do ... -- Sebastian Schuberth ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-26 10:24 ` Adding Beyond Compare as a merge tool, was: " Sebastian Schuberth 2011-02-26 10:33 ` Junio C Hamano @ 2011-02-27 5:38 ` Chris Packham 2011-02-27 9:09 ` Junio C Hamano 2011-02-27 10:57 ` Sebastian Schuberth 1 sibling, 2 replies; 46+ messages in thread From: Chris Packham @ 2011-02-27 5:38 UTC (permalink / raw) To: git; +Cc: Sebastian Schuberth, Junio C Hamano, charles On 26/02/11 23:24, Sebastian Schuberth wrote: > On 24.02.2011 00:26, Junio C Hamano wrote: > >> * cp/mergetool-beyondcompare (2011-02-18) 1 commit >> - mergetool--lib: add support for beyond compare > > Sorry for not responding earlier to this, but problems at my news > provider seem to have swallowed mails from several days, including the > original post of > > http://marc.info/?l=git&m=129801656713478&w=2 > > A while ago, I had already proposed > > http://marc.info/?l=git&m=129007741814521&w=2 > > I'm not entirely sure why it was ignored in the end, probably I did not > report back to have tested it in Linux. Sorry that I missed it (I didn't really look too hard). > > A few things that I like better in my patch than in Chris': > > - Beyond Compare is added as "bc3" instead of "bcompare", which is both > shorter and indicates that only version 3, not version 2, is supported. v2 isn't natively available for linux, although it works quite well under wine. Either way you're quite correct that v2 is not much use as a 3-way merge tool. Trying to indicate v3 only make sense to me (at least until they release a v4 :) > - Chris seems to be missing the patch to git-gui/lib/mergetool.tcl Didn't know to add it, thanks for pointing it out. If my patch get's picked up I'll include it in the next round. > - To the best of my knownledge, the Beyond Compare executable is called > "BCompare" (note the case), that means even with the merge tool named > "bcompare" a translation step in git-mergetool--lib.sh should by > required (as done in my patch). Chris, as you seem to have tested ion > Linux, could you shed a light on this? In linux it's bcompare, although BCompare is the eventual executable chrisp@laptop:~> rpm -q --filesbypkg bcompare bcompare /usr/bin/bcompare bcompare /usr/lib/beyondcompare/BCompare Unfortunately /usr/bin/bcompare is a little more involved than a symlink so for linux we need to call bcompare. Do we have a nice way of handling this? We could just treat them completely separate but that seems a but like sweeping the problem under the carpet. > - Using dashes for the options to Beyond Compare is fine on Windows, Good to hear. I was meaning to fire up a vbox windows XP image to double check but you've saved me the hassle. > however, I believe the order of the files is wrong, although that might > be a bit subjective. For a 3-way merge the syntax is > > BCompare.exe C:\Left.ext C:\Right.ext C:\Center.ext > > So the file that should go to the center panel is specified last. AFAIK, > all other merge tools are called such that $BASE goes to the center. > This is why my patch specifies $BASE last. I actually prefer base on the left a-la kdiff3 and araxis (and from my previous life as a clearcase user). I've since found that with beyond compare it is a pain to take changes from the center version so I agree with your suggestion. We should probably make use of the -title options to remove ambiguity. > Any more opinions? Chris, in case you'd agree to prefer my patch, I'd be > very grateful if you could test it on Linux. > > For your convenience, I've rebased onto the current master and attached > the patch files. I'll give it a whirl when I get a chance. I suspect I'll need to handle the BCompare/bcompare thing but that's not a huge issue. ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-27 5:38 ` Chris Packham @ 2011-02-27 9:09 ` Junio C Hamano 2011-02-27 10:58 ` Sebastian Schuberth 2011-02-27 10:57 ` Sebastian Schuberth 1 sibling, 1 reply; 46+ messages in thread From: Junio C Hamano @ 2011-02-27 9:09 UTC (permalink / raw) To: Chris Packham; +Cc: git, Sebastian Schuberth, charles Chris Packham <judge.packham@gmail.com> writes: > In linux it's bcompare, although BCompare is the eventual executable > > chrisp@laptop:~> rpm -q --filesbypkg bcompare > bcompare /usr/bin/bcompare > bcompare /usr/lib/beyondcompare/BCompare > > Unfortunately /usr/bin/bcompare is a little more involved than a symlink > so for linux we need to call bcompare. The real question is which one the end-users like git-mergetool--lib are expected to call, and it looks like the "bcompare" in /usr/bin spelled in lowercase is the one. So shouldn't we be calling that, Sebastian? And if we invoke lowercase "bcompare" on Windows, the system would do the right thing, no? ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-27 9:09 ` Junio C Hamano @ 2011-02-27 10:58 ` Sebastian Schuberth 0 siblings, 0 replies; 46+ messages in thread From: Sebastian Schuberth @ 2011-02-27 10:58 UTC (permalink / raw) To: Junio C Hamano; +Cc: Chris Packham, git, charles On Sun, Feb 27, 2011 at 10:09, Junio C Hamano <gitster@pobox.com> wrote: >> Unfortunately /usr/bin/bcompare is a little more involved than a symlink >> so for linux we need to call bcompare. > > The real question is which one the end-users like git-mergetool--lib are > expected to call, and it looks like the "bcompare" in /usr/bin spelled in > lowercase is the one. So shouldn't we be calling that, Sebastian? > > And if we invoke lowercase "bcompare" on Windows, the system would do the > right thing, no? Yes and yes, I'll update my patch accordingly. -- Sebastian Schuberth ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-27 5:38 ` Chris Packham 2011-02-27 9:09 ` Junio C Hamano @ 2011-02-27 10:57 ` Sebastian Schuberth 2011-02-27 11:15 ` Sebastian Schuberth 1 sibling, 1 reply; 46+ messages in thread From: Sebastian Schuberth @ 2011-02-27 10:57 UTC (permalink / raw) To: Chris Packham; +Cc: git, Junio C Hamano, charles On Sun, Feb 27, 2011 at 06:38, Chris Packham <judge.packham@gmail.com> wrote: > Sorry that I missed it (I didn't really look too hard). No problem, at least this got me dig out my series again ;-) > 3-way merge tool. Trying to indicate v3 only make sense to me (at least > until they release a v4 :) True. In that case, read it as "Beyond Compare version 3 an up" ;-) > In linux it's bcompare, although BCompare is the eventual executable > > chrisp@laptop:~> rpm -q --filesbypkg bcompare > bcompare /usr/bin/bcompare > bcompare /usr/lib/beyondcompare/BCompare Ah, OK, being on Windows I just checked the TAR.GZ file, which does not contain /usr/bin/bcompare (but bcompare.sh, which probably gets installed by install.sh). > Unfortunately /usr/bin/bcompare is a little more involved than a symlink > so for linux we need to call bcompare. Do we have a nice way of handling > this? We could just treat them completely separate but that seems a but > like sweeping the problem under the carpet. We could just call "bcompare" on Windows, too. As Windows is case-insensitive, it will just work. > with your suggestion. We should probably make use of the -title options > to remove ambiguity. Good idea, I'll look into this. > I'll give it a whirl when I get a chance. I suspect I'll need to handle > the BCompare/bcompare thing but that's not a huge issue. You could probably just wait a few minutes until I have pushed out an update for this patch. -- Sebastian Schuberth ^ permalink raw reply [flat|nested] 46+ messages in thread
* Re: Adding Beyond Compare as a merge tool, was: Re: What's cooking in git.git (Feb 2011, #05; Wed, 23) 2011-02-27 10:57 ` Sebastian Schuberth @ 2011-02-27 11:15 ` Sebastian Schuberth 0 siblings, 0 replies; 46+ messages in thread From: Sebastian Schuberth @ 2011-02-27 11:15 UTC (permalink / raw) To: Chris Packham; +Cc: git, Junio C Hamano, charles On Sun, Feb 27, 2011 at 11:57, Sebastian Schuberth <sschuberth@gmail.com> wrote: >> with your suggestion. We should probably make use of the -title options >> to remove ambiguity. > > Good idea, I'll look into this. Given that the -title options actually replace the display of the file name (instead of displaying the title in addition), I'd prefer not to make use of these options. In fact, the file names Git uses for merging already contain "BASE", "LOCAL", "REMOTE", so there should be no real need for additional titles, IMHO. I'll push out the updated series now. -- Sebastian Schuberth ^ permalink raw reply [flat|nested] 46+ messages in thread
end of thread, other threads:[~2011-03-12 15:38 UTC | newest] Thread overview: 46+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-02-23 23:26 What's cooking in git.git (Feb 2011, #05; Wed, 23) Junio C Hamano 2011-02-23 23:48 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Jonathan Nieder 2011-02-24 1:16 ` Junio C Hamano 2011-02-24 2:55 ` Ævar Arnfjörð Bjarmason 2011-02-24 3:14 ` Jonathan Nieder 2011-02-24 10:38 ` Ævar Arnfjörð Bjarmason 2011-02-24 10:45 ` Jonathan Nieder 2011-02-24 11:00 ` Jonathan Nieder 2011-02-25 21:48 ` Ævar Arnfjörð Bjarmason 2011-02-26 5:14 ` Jonathan Nieder 2011-02-24 4:00 ` ab/i18n Jonathan Nieder 2011-02-24 9:56 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Erik Faye-Lund 2011-02-24 10:27 ` ab/i18n Jonathan Nieder 2011-02-24 19:11 ` ab/i18n Junio C Hamano 2011-02-24 10:40 ` ab/i18n (What's cooking in git.git (Feb 2011, #05; Wed, 23)) Ævar Arnfjörð Bjarmason 2011-02-23 23:52 ` What's cooking in git.git (Feb 2011, #05; Wed, 23) Johan Herland 2011-02-24 0:43 ` Junio C Hamano 2011-02-24 1:04 ` Johan Herland 2011-02-24 0:32 ` Ævar Arnfjörð Bjarmason 2011-02-24 0:57 ` Junio C Hamano 2011-02-24 1:37 ` Ævar Arnfjörð Bjarmason 2011-02-24 1:59 ` Junio C Hamano 2011-02-24 2:07 ` Ævar Arnfjörð Bjarmason 2011-02-24 9:45 ` Erik Faye-Lund 2011-02-24 17:47 ` Ævar Arnfjörð Bjarmason 2011-02-24 22:39 ` Jonathan Nieder 2011-02-25 1:45 ` [msysGit] " Johannes Schindelin 2011-02-25 9:05 ` Kirill Smelkov 2011-02-25 11:11 ` Johannes Schindelin 2011-02-25 19:24 ` Kirill Smelkov 2011-02-25 21:54 ` Johannes Schindelin 2011-02-26 11:07 ` Kirill Smelkov 2011-02-26 19:07 ` Kirill Smelkov 2011-02-26 19:58 ` cross-compiling msys-1.0.dll, was " Johannes Schindelin 2011-03-12 13:18 ` Kirill Smelkov 2011-03-12 15:38 ` Johannes Schindelin 2011-02-27 14:28 ` Johannes Schindelin 2011-03-12 13:16 ` Kirill Smelkov 2011-02-26 10:24 ` Adding Beyond Compare as a merge tool, was: " Sebastian Schuberth 2011-02-26 10:33 ` Junio C Hamano 2011-02-26 10:48 ` Sebastian Schuberth 2011-02-27 5:38 ` Chris Packham 2011-02-27 9:09 ` Junio C Hamano 2011-02-27 10:58 ` Sebastian Schuberth 2011-02-27 10:57 ` Sebastian Schuberth 2011-02-27 11:15 ` Sebastian Schuberth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).