* Re: [PATCH v2] completion: fix issue with process substitution not working on Git for Windows
From: Stefan Näwe @ 2011-10-27 6:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: spearce, git
In-Reply-To: <7vipnb1myv.fsf@alter.siamese.dyndns.org>
Am 26. Oktober 2011 23:07 schrieb Junio C Hamano <gitster@pobox.com>:
> Stefan Naewe <stefan.naewe@gmail.com> writes:
>
>> $ export GIT_PS1_SHOWUPSTREAM=1
>> sh.exe": cannot make pipe for process substitution: Function not implemented
>> sh.exe": cannot make pipe for process substitution: Function not implemented
>> sh.exe": <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n '): ambiguous redirect
>
> Are these the exact strings you want to have in the commit log message? I
> am particularly wondering about the dq after (but not around) 'sh.exe'.
Yes. That's exactly what I get.
Stefan
--
----------------------------------------------------------------
python -c "print '73746566616e2e6e6165776540676d61696c2e636f6d'.decode('hex')"
^ permalink raw reply
* What's cooking in git.git (Oct 2011, #10; Wed, 26)
From: Junio C Hamano @ 2011-10-27 0:40 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with '-' are
only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'.
It probably is a good point to stop taking new topics and start
switching our focus to fixing bugs in the topics already in 'master'.
Here are the repositories that have my integration branches:
With maint, master, next, pu and todo:
git://git.kernel.org/pub/scm/git/git.git
git://repo.or.cz/alt-git.git
https://code.google.com/p/git-core/
https://github.com/git/git
With only maint and master:
git://git.sourceforge.jp/gitroot/git-core/git.git
git://git-core.git.sourceforge.net/gitroot/git-core/git-core
With all the topics and integration branches but not todo, html or man:
https://github.com/gitster/git
I will stop pushing the generated documentation branches to the above
repositories, as they are not sources. The only reason the source
repository at k.org has hosted these branches was because it was the only
repository over there that was writable by me; it was an ugly historical
and administrative workaround and not a demonstration of the best
practice.
These branches are pushed to their own separate repositories instead:
git://git.kernel.org/pub/scm/git/git-{htmldocs,manpages}.git/
git://repo.or.cz/git-{htmldocs,manpages}.git/
https://code.google.com/p/git-{htmldocs,manpages}.git/
https://github.com/gitster/git-{htmldocs,manpages}.git/
--------------------------------------------------
[New Topics]
* ef/mingw-upload-archive (2011-10-26) 3 commits
- upload-archive: use start_command instead of fork
- compat/win32/poll.c: upgrade from upstream
- mingw: move poll out of sys-folder
* js/grep-mutex (2011-10-26) 3 commits
(merged to 'next' on 2011-10-26 at 6fac2d6)
+ builtin/grep: simplify lock_and_read_sha1_file()
+ builtin/grep: make lock/unlock into static inline functions
+ git grep: be careful to use mutexes only when they are initialized
Will merge to "master" shortly.
* rj/gitweb-clean-js (2011-10-26) 1 commit
(merged to 'next' on 2011-10-26 at db36a24)
+ gitweb/Makefile: Remove static/gitweb.js in the clean target
Will merge to "master" shortly.
* rs/allocate-cache-entry-individually (2011-10-26) 2 commits
- cache.h: put single NUL at end of struct cache_entry
- read-cache.c: allocate index entries individually
* rs/maint-estimate-cache-size (2011-10-26) 1 commit
(merged to 'next' on 2011-10-26 at 2f11375)
+ read-cache.c: fix index memory allocation
Will merge to "master" shortly.
* sn/complete-bash-wo-process-subst (2011-10-26) 1 commit
(merged to 'next' on 2011-10-26 at 8662ed6)
+ completion: fix issue with process substitution not working on Git for Windows
Will merge to "master" shortly.
--------------------------------------------------
[Graduated to "master"]
* cn/fetch-prune (2011-10-15) 5 commits
(merged to 'next' on 2011-10-16 at 02a449e)
+ fetch: treat --tags like refs/tags/*:refs/tags/* when pruning
+ fetch: honor the user-provided refspecs when pruning refs
+ remote: separate out the remote_find_tracking logic into query_refspecs
+ t5510: add tests for fetch --prune
+ fetch: free all the additional refspecs
"git fetch --prune" used to prune remote tracking branches by comparing
what was actually fetched and what was configured to be fetched, which was
wrong.
* jm/maint-gitweb-filter-forks-fix (2011-10-21) 1 commit
(merged to 'next' on 2011-10-21 at debedcd)
+ gitweb: fix regression when filtering out forks
* jn/libperl-git-config (2011-10-21) 2 commits
(merged to 'next' on 2011-10-21 at 76e2d4b)
+ Add simple test for Git::config_path() in t/t9700-perl-git.sh
+ libperl-git: refactor Git::config_*
* lh/gitweb-site-html-head (2011-10-21) 1 commit
(merged to 'next' on 2011-10-23 at 65075df)
+ gitweb: provide a way to customize html headers
* mm/mediawiki-author-fix (2011-10-20) 1 commit
(merged to 'next' on 2011-10-23 at 9f85b67)
+ git-remote-mediawiki: don't include HTTP login/password in author
* tc/submodule-clone-name-detection (2011-10-21) 2 commits
(merged to 'next' on 2011-10-23 at c18af03)
+ submodule::module_clone(): silence die() message from module_name()
+ submodule: whitespace fix
"git submodule clone" used to show unnecessary error message when
submodule mapping from name to path is not found in .gitmodules file.
--------------------------------------------------
[Stalled]
* hv/submodule-merge-search (2011-10-13) 4 commits
- submodule.c: make two functions static
- allow multiple calls to submodule merge search for the same path
- push: Don't push a repository with unpushed submodules
- push: teach --recurse-submodules the on-demand option
What the topic aims to achieve may make sense, but the implementation
looked somewhat suboptimal.
* sr/transport-helper-fix-rfc (2011-07-19) 2 commits
- t5800: point out that deleting branches does not work
- t5800: document inability to push new branch with old content
Perhaps 281eee4 (revision: keep track of the end-user input from the
command line, 2011-08-25) would help.
* jc/lookup-object-hash (2011-08-11) 6 commits
- object hash: replace linear probing with 4-way cuckoo hashing
- object hash: we know the table size is a power of two
- object hash: next_size() helper for readability
- pack-objects --count-only
- object.c: remove duplicated code for object hashing
- object.c: code movement for readability
I do not think there is anything fundamentally wrong with this series, but
the risk of breakage far outweighs observed performance gain in one
particular workload.
* jc/verbose-checkout (2011-10-16) 2 commits
- checkout -v: give full status output after switching branches
- checkout: move the local changes report to the end
This is just to leave a record that the reason why we do not do this not
because we are incapable of coding this, but because it is not a good idea
to do this. I suspect people who are new to git that might think they need
it would soon realize the don't.
Will keep in 'pu' as a showcase for a while and then will drop.
* kk/gitweb-side-by-side-diff (2011-10-17) 2 commits
- gitweb: add a feature to show side-by-side diff
- gitweb: change format_diff_line() to remove leading SP from $diff_class
Fun.
Will keep in 'pu' until the planned re-roll comes.
--------------------------------------------------
[Cooking]
* nd/pretty-commit-log-message (2011-10-23) 2 commits
- pretty.c: use original commit message if reencoding fails
- pretty.c: free get_header() return value
* mh/ref-api-3 (2011-10-19) 11 commits
(merged to 'next' on 2011-10-23 at 92e2d35)
+ is_refname_available(): reimplement using do_for_each_ref_in_array()
+ names_conflict(): simplify implementation
+ names_conflict(): new function, extracted from is_refname_available()
+ repack_without_ref(): reimplement using do_for_each_ref_in_array()
+ do_for_each_ref_in_array(): new function
+ do_for_each_ref(): correctly terminate while processesing extra_refs
+ add_ref(): take a (struct ref_entry *) parameter
+ create_ref_entry(): extract function from add_ref()
+ parse_ref_line(): add a check that the refname is properly formatted
+ repack_without_ref(): remove temporary
+ Rename another local variable name -> refname
(this branch uses mh/ref-api-2.)
* rr/revert-cherry-pick (2011-10-23) 5 commits
(merged to 'next' on 2011-10-26 at 27b7496)
+ revert: simplify communicating command-line arguments
+ revert: allow mixed pick and revert instructions
+ revert: make commit subjects in insn sheet optional
+ revert: simplify getting commit subject in format_todo()
+ revert: free msg in format_todo()
The internals of "git revert/cherry-pick" has been further refactored to
serve as the basis for the sequencer.
Will keep in 'next' during this cycle.
* jc/check-ref-format-fixup (2011-10-19) 2 commits
(merged to 'next' on 2011-10-19 at 98981be)
+ Revert "Restrict ref-like names immediately below $GIT_DIR"
(merged to 'next' on 2011-10-15 at 8e89bc5)
+ Restrict ref-like names immediately below $GIT_DIR
This became a no-op except for the bottom one which is part of the other
topic now.
Will discard once the other topic graduates to 'master'.
* cb/daemon-permission-errors (2011-10-17) 2 commits
- daemon: report permission denied error to clients
- daemon: add tests
The tip commit might be loosening things a bit too much.
Will keep in 'pu' until hearing a convincing argument for the patch.
* mh/ref-api-2 (2011-10-17) 14 commits
(merged to 'next' on 2011-10-19 at cc89f0e)
+ resolve_gitlink_ref_recursive(): change to work with struct ref_cache
+ Pass a (ref_cache *) to the resolve_gitlink_*() helper functions
+ resolve_gitlink_ref(): improve docstring
+ get_ref_dir(): change signature
+ refs: change signatures of get_packed_refs() and get_loose_refs()
+ is_dup_ref(): extract function from sort_ref_array()
+ add_ref(): add docstring
+ parse_ref_line(): add docstring
+ is_refname_available(): remove the "quiet" argument
+ clear_ref_array(): rename from free_ref_array()
+ refs: rename parameters result -> sha1
+ refs: rename "refname" variables
+ struct ref_entry: document name member
+ cache.h: add comments for git_path() and git_path_submodule()
(this branch is used by mh/ref-api-3.)
It is either merge this quickly to 'master' and hope there won't be any
more unexpected breakage that forces us to delay the release, or hold it
on 'next' until the next cycle. I am inclined to do the former, but not
quite ready to commit to it yet.
* dm/pack-objects-update (2011-10-20) 4 commits
- pack-objects: don't traverse objects unnecessarily
- pack-objects: rewrite add_descendants_to_write_order() iteratively
- pack-objects: use unsigned int for counter and offset values
- pack-objects: mark add_to_write_order() as inline
Need to re-read this before deciding what to do; it came a bit too late in
the cycle for a series that touches a seriously important part of the
system.
* jk/git-tricks (2011-10-21) 3 commits
(merged to 'next' on 2011-10-23 at 7c9bf71)
+ completion: match ctags symbol names in grep patterns
+ contrib: add git-jump script
+ contrib: add diff highlight script
As this stuff is in contrib/ I do not care too much about the stability.
Will merge to 'master' unless there is strong objection.
* jc/signed-commit (2011-10-21) 7 commits
(merged to 'next' on 2011-10-23 at 03eec25)
+ pretty: %G[?GS] placeholders
+ parse_signed_commit: really use the entire commit log message
+ test "commit -S" and "log --show-signature"
+ t7004: extract generic "GPG testing" bits
+ log: --show-signature
+ commit: teach --gpg-sign option
+ Split GPG interface into its own helper library
This is to replace the earlier "signed push" experiments.
Will keep in 'next' during this cycle.
* sg/complete-refs (2011-10-21) 9 commits
(merged to 'next' on 2011-10-26 at d65e2b4)
+ completion: remove broken dead code from __git_heads() and __git_tags()
+ completion: fast initial completion for config 'remote.*.fetch' value
+ completion: improve ls-remote output filtering in __git_refs_remotes()
+ completion: query only refs/heads/ in __git_refs_remotes()
+ completion: support full refs from remote repositories
+ completion: improve ls-remote output filtering in __git_refs()
+ completion: make refs completion consistent for local and remote repos
+ completion: optimize refs completion
+ completion: document __gitcomp()
Will keep in 'next' until an Ack or two from completion folks.
* jc/request-pull-show-head-4 (2011-10-15) 11 commits
(merged to 'next' on 2011-10-15 at 7e340ff)
+ fmt-merge-msg.c: Fix an "dubious one-bit signed bitfield" sparse error
(merged to 'next' on 2011-10-10 at 092175e)
+ environment.c: Fix an sparse "symbol not declared" warning
+ builtin/log.c: Fix an "Using plain integer as NULL pointer" warning
(merged to 'next' on 2011-10-07 at fcaeca0)
+ fmt-merge-msg: use branch.$name.description
(merged to 'next' on 2011-10-06 at fa5e0fe)
+ request-pull: use the branch description
+ request-pull: state what commit to expect
+ request-pull: modernize style
+ branch: teach --edit-description option
+ format-patch: use branch description in cover letter
+ branch: add read_branch_desc() helper function
+ Merge branch 'bk/ancestry-path' into jc/branch-desc
Allow setting "description" for branches and use it to help communications
between humans in various workflow elements.
Will keep in 'next' during this cycle.
^ permalink raw reply
* Re: [PATCH] replace sha1 with another algorithm
From: Jeff King @ 2011-10-27 0:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaa8n35dc.fsf@alter.siamese.dyndns.org>
On Wed, Oct 26, 2011 at 12:44:15PM -0700, Junio C Hamano wrote:
> > +static void mix_hash(unsigned char *h, unsigned n)
> > +{
> > + unsigned char out[20];
> > + unsigned mid = n / 2;
> > +
> > + if (2*mid < n)
> > + return;
> > +
> > + xor_bytes(out, h, h + mid, mid);
> > + xor_bytes(out + mid, h + mid, h, mid);
> > + memcpy(h, out, n);
> > +
> > + /* If a little bit of mixing is good, then a lot must be GREAT! */
> > + mix_hash(h, mid);
> > + mix_hash(h + mid, mid);
> > +}
>
> You seem to want to reduce the hash down to 5-bytes by duplicating the
> same value on the left and right half, and duplicate that four times to
> fill 20-byte buffer, but doesn't this look unnecessarily inefficient way
> to achieve that?
Well, yeah. But when you're writing a really bad hashing algorithm, I
feel like obfuscating the bugs is key.
-Peff
^ permalink raw reply
* Problem with git svn clone --authors-file
From: edman747 @ 2011-10-26 23:51 UTC (permalink / raw)
To: git
Hello,
Attempting to clone a remote svn repo where I don't know all the
previous SVN author names.
installed msysgit (vista)
gitbash,
$ mkdir test
$ cd test
create authors file with a few known authors.
$ git svn clone --authors-file=authors http://svn.repo/trunk
...
runs fine until
Author: (no author) not defined in authors file
edit authors file add line: (no author) = none <email>
------
rerun previous git svn command
$ git svn clone --authors-file=authors http://svn.repo/trunk
Using existing [svn-remote "svn"]
svn-remote.svn.fetch already set to track :refs/remotes/git-svn
It stops.
$ vi trunk/.git/config
delete line: fetch = :refs/remotes/git-svn
-----
rerun previous git svn command
$ git svn clone --authors-file=authors http://svn.repo/trunk
...
runs fine until
Author: Bob not defined in authors file
edit authors file add line: Bob = bobby jones <email>
-----
rerun previous git svn command
$ git svn clone --authors-file=authors http://svn.repo/trunk
Using existing [svn-remote "svn"]
svn-remote.svn.fetch already set to track :refs/remotes/git-svn
Each time it encounters an SVN committer name that is not in the
authors-file it aborts. As expected, edit the authors file and re-run
the previous git svn command to continue
And it quits with
svn-remote.svn.fetch already set to track :refs/remotes/git-svn
Would like for it to continue without having to edit the trunk/.get/config file.
Did I miss a flag or option?
Thank You,
>From git-svn(1)
--authors-file=<filename>
Syntx is compatible with the file used by git cvsimport:
loginname
If this option is specified and git svn encounters an SVN committer
name that does not exist in the authors-file, git svn will abort
operation. The user will then have to add the appropriate entry.
Re-running the previous git svn command after the authors-file is
modified should continue operation.
^ permalink raw reply
* Re: [PATCH v2] completion: fix issue with process substitution not working on Git for Windows
From: Junio C Hamano @ 2011-10-26 21:07 UTC (permalink / raw)
To: Stefan Naewe; +Cc: spearce, git
In-Reply-To: <1319656389-9515-1-git-send-email-stefan.naewe@gmail.com>
Stefan Naewe <stefan.naewe@gmail.com> writes:
> $ export GIT_PS1_SHOWUPSTREAM=1
> sh.exe": cannot make pipe for process substitution: Function not implemented
> sh.exe": cannot make pipe for process substitution: Function not implemented
> sh.exe": <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n '): ambiguous redirect
Are these the exact strings you want to have in the commit log message? I
am particularly wondering about the dq after (but not around) 'sh.exe'.
^ permalink raw reply
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Junio C Hamano @ 2011-10-26 20:08 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, msysgit
In-Reply-To: <7v39ef34in.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> The remainder of this message are hints and random thoughts on potential
> follow-up patches that may want to build on top of this patch for further
> clean-ups (not specifically meant for Dscho but for other people on both
> mailing lists).
> ...
> - Could we lose "#ifndef NO_PTHREADS" inside grep_sha1(), grep_file(),
> and possibly cmd_grep() functions and let the compiler optimize things
> away under NO_PTHREADS compilation?
I suspect that the result of the conversion would look a lot cleaner if
the code is first cleaned up to move global variable like skip_first_line
and the mutexes into the grep_opt structure. Without such clean-up, I do
not think a conversion like this does not add much value.
But since I already did it,...
-- >8 --
Subject: [PATCH] builtin/grep: war on #if[n]def inside function body
Get rid of #if[n]def inside implementation of the function body
and let the compiler optimize codepaths that are protected with
"if (use_threads)" away on NO_PTHREADS builds.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/grep.c | 37 +++++++++++++++++++------------------
1 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 3d7329d..f24f3a7 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -24,9 +24,13 @@ static char const * const grep_usage[] = {
NULL
};
-static int use_threads = 1;
+/* Skip the leading hunk mark of the first file. */
+static int skip_first_line;
#ifndef NO_PTHREADS
+static int use_threads = 1;
+#define no_threads() use_threads = 0
+
#define THREADS 8
static pthread_t threads[THREADS];
@@ -112,8 +116,6 @@ static pthread_cond_t cond_write;
/* Signalled when we are finished with everything. */
static pthread_cond_t cond_result;
-static int skip_first_line;
-
static void add_work(enum work_type type, char *name, void *id)
{
grep_lock();
@@ -181,7 +183,6 @@ static void work_done(struct work_item *w)
const char *p = w->out.buf;
size_t len = w->out.len;
- /* Skip the leading hunk mark of the first file. */
if (skip_first_line) {
while (len) {
len--;
@@ -310,8 +311,18 @@ static int wait_all(void)
return hit;
}
#else /* !NO_PTHREADS */
+#define use_threads 0
+#define no_threads() /* noop */
+
#define read_sha1_lock()
#define read_sha1_unlock()
+#define grep_lock()
+#define grep_unlock()
+
+#define online_cpus() 1
+#define grep_sha1_async(opt, name, sha1)
+#define grep_file_async(opt, name, filename)
+#define start_threads(opt)
static int wait_all(void)
{
@@ -407,13 +418,10 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
name = strbuf_detach(&pathbuf, NULL);
-#ifndef NO_PTHREADS
if (use_threads) {
grep_sha1_async(opt, name, sha1);
return 0;
- } else
-#endif
- {
+ } else {
int hit;
unsigned long sz;
void *data = load_sha1(sha1, &sz, name);
@@ -469,13 +477,10 @@ static int grep_file(struct grep_opt *opt, const char *filename)
strbuf_addstr(&buf, filename);
name = strbuf_detach(&buf, NULL);
-#ifndef NO_PTHREADS
if (use_threads) {
grep_file_async(opt, name, filename);
return 0;
- } else
-#endif
- {
+ } else {
int hit;
size_t sz;
void *data = load_file(filename, &sz);
@@ -992,7 +997,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
opt.output_priv = &path_list;
opt.output = append_path;
string_list_append(&path_list, show_in_pager);
- use_threads = 0;
+ no_threads();
}
if (!opt.pattern_list)
@@ -1000,9 +1005,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (!opt.fixed && opt.ignore_case)
opt.regflags |= REG_ICASE;
-#ifndef NO_PTHREADS
if (online_cpus() == 1 || !grep_threads_ok(&opt))
- use_threads = 0;
+ no_threads();
if (use_threads) {
if (opt.pre_context || opt.post_context || opt.file_break ||
@@ -1010,9 +1014,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
skip_first_line = 1;
start_threads(&opt);
}
-#else
- use_threads = 0;
-#endif
compile_grep_patterns(&opt);
--
1.7.7.1.504.gcc718
^ permalink raw reply related
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Junio C Hamano @ 2011-10-26 20:04 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, msysgit
In-Reply-To: <7v39ef34in.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> The remainder of this message are hints and random thoughts on potential
> follow-up patches that may want to build on top of this patch for further
> clean-ups (not specifically meant for Dscho but for other people on both
> mailing lists).
> ...
> - Wouldn't the result be more readable to make these into static inline
> functions?
That would look like this.
-- >8 --
Subject: [PATCH] builtin/grep: make lock/unlock into static inline
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/grep.c | 28 +++++++++++++++++++++++-----
1 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 88b0c80..3ddfae4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -74,14 +74,32 @@ static int all_work_added;
/* This lock protects all the variables above. */
static pthread_mutex_t grep_mutex;
+static inline void grep_lock(void)
+{
+ if (use_threads)
+ pthread_mutex_lock(&grep_mutex);
+}
+
+static inline void grep_unlock(void)
+{
+ if (use_threads)
+ pthread_mutex_unlock(&grep_mutex);
+}
+
/* Used to serialize calls to read_sha1_file. */
static pthread_mutex_t read_sha1_mutex;
-#define WHEN_THREADED(x) do { if (use_threads) (x); } while (0)
-#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex))
-#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex))
-#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex))
-#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex))
+static inline void read_sha1_lock(void)
+{
+ if (use_threads)
+ pthread_mutex_lock(&read_sha1_mutex);
+}
+
+static inline void read_sha1_unlock(void)
+{
+ if (use_threads)
+ pthread_mutex_unlock(&read_sha1_mutex);
+}
/* Signalled when a new work_item is added to todo. */
static pthread_cond_t cond_add;
--
1.7.7.1.504.gcc718
^ permalink raw reply related
* Re: [PATCH v2] completion: fix issue with process substitution not working on Git for Windows
From: Junio C Hamano @ 2011-10-26 20:02 UTC (permalink / raw)
To: Stefan Naewe; +Cc: spearce, git
In-Reply-To: <1319656389-9515-1-git-send-email-stefan.naewe@gmail.com>
Stefan Naewe <stefan.naewe@gmail.com> writes:
> Git for Windows comes with a bash that doesn't support process substitution.
> It issues the following error when using git-completion.bash with
> GIT_PS1_SHOWUPSTREAM set:
>
> $ export GIT_PS1_SHOWUPSTREAM=1
> sh.exe": cannot make pipe for process substitution: Function not implemented
> sh.exe": cannot make pipe for process substitution: Function not implemented
> sh.exe": <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n '): ambiguous redirect
>
> Replace the process substitution with a 'here string'.
>
> Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
Yuck, but I honestly shouldn't care about the yuckiness as this script is
inherently intimately dependent on bash anyway ;-).
> contrib/completion/git-completion.bash | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8648a36..0b3d47e 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -110,6 +110,7 @@ __git_ps1_show_upstream ()
> local upstream=git legacy="" verbose=""
>
> # get some config options from git-config
> + output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
> while read key value; do
> case "$key" in
> bash.showupstream)
> @@ -125,7 +126,7 @@ __git_ps1_show_upstream ()
> upstream=svn+git # default upstream is SVN if available, else git
> ;;
> esac
> - done < <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')
> + done <<< "$output"
>
> # parse configuration values
> for option in ${GIT_PS1_SHOWUPSTREAM}; do
^ permalink raw reply
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Junio C Hamano @ 2011-10-26 20:02 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: msysgit, git
In-Reply-To: <alpine.DEB.1.00.1110251223500.32316@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> I looked around a bit but ran out of time to identify the reason why
> this was not caught earlier.
>
> builtin/grep.c | 9 +++++----
> 1 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 92eeada..e94c5fe 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -78,10 +78,11 @@ static pthread_mutex_t grep_mutex;
> /* Used to serialize calls to read_sha1_file. */
> static pthread_mutex_t read_sha1_mutex;
>
> -#define grep_lock() pthread_mutex_lock(&grep_mutex)
> -#define grep_unlock() pthread_mutex_unlock(&grep_mutex)
> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)
> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)
> +#define WHEN_THREADED(x) do { if (use_threads) (x); } while (0)
> +#define grep_lock() WHEN_THREADED(pthread_mutex_lock(&grep_mutex))
> +#define grep_unlock() WHEN_THREADED(pthread_mutex_unlock(&grep_mutex))
> +#define read_sha1_lock() WHEN_THREADED(pthread_mutex_lock(&read_sha1_mutex))
> +#define read_sha1_unlock() WHEN_THREADED(pthread_mutex_unlock(&read_sha1_mutex))
I think, from a quick glance, this is a good first step.
The remainder of this message are hints and random thoughts on potential
follow-up patches that may want to build on top of this patch for further
clean-ups (not specifically meant for Dscho but for other people on both
mailing lists).
- The patch makes the check for use_threads in lock_and_read_sha1_file()
redundant. The other user of read_sha1_lock/unlock in grep_object() can
take advantage of this change (see below).
- It makes me wonder if it is simpler to initialize mutexes even in
!use_threads case.
- Wouldn't the result be more readable to make these into static inline
functions?
- Could we lose "#ifndef NO_PTHREADS" inside grep_sha1(), grep_file(),
and possibly cmd_grep() functions and let the compiler optimize things
away under NO_PTHREADS compilation?
diff --git a/builtin/grep.c b/builtin/grep.c
index 7d0779f..60daa85 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -354,13 +354,9 @@ static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type
{
void *data;
- if (use_threads) {
- read_sha1_lock();
- data = read_sha1_file(sha1, type, size);
- read_sha1_unlock();
- } else {
- data = read_sha1_file(sha1, type, size);
- }
+ read_sha1_lock();
+ data = read_sha1_file(sha1, type, size);
+ read_sha1_unlock();
return data;
}
^ permalink raw reply related
* Re: [PATCH] replace sha1 with another algorithm
From: Junio C Hamano @ 2011-10-26 19:44 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111026001237.GA22195@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> +static void xor_bytes(unsigned char *out, unsigned char *a, unsigned char *b,
> + unsigned n)
> +{
> + unsigned i;
> + for (i = 0; i < n; i++)
> + out[i] = a[i] ^ b[i];
> +}
> +
> +static void mix_hash(unsigned char *h, unsigned n)
> +{
> + unsigned char out[20];
> + unsigned mid = n / 2;
> +
> + if (2*mid < n)
> + return;
> +
> + xor_bytes(out, h, h + mid, mid);
> + xor_bytes(out + mid, h + mid, h, mid);
> + memcpy(h, out, n);
> +
> + /* If a little bit of mixing is good, then a lot must be GREAT! */
> + mix_hash(h, mid);
> + mix_hash(h + mid, mid);
> +}
You seem to want to reduce the hash down to 5-bytes by duplicating the
same value on the left and right half, and duplicate that four times to
fill 20-byte buffer, but doesn't this look unnecessarily inefficient way
to achieve that?
^ permalink raw reply
* Re: git-fixup-assigner.perl -- automatically decide where to "fixup!"
From: Thomas Rast @ 2011-10-26 19:40 UTC (permalink / raw)
To: fREW Schmidt; +Cc: git
In-Reply-To: <CADVrmKT1woYpJoe=L9sAbQ30TUw44zMc7y4WF=PMrT5Gj9kDNw@mail.gmail.com>
fREW Schmidt wrote:
>
> $ git fixup-assigner.pl > fixups && less fixups
> #!/bin/sh
>
> git apply --cached <<EOF
[...]
> - $self->set($c,
> $c->model('DB')->schema->kiokudb_handle->lookup('dashboard
> templates'));
> + $self->set($c, $c->model('Kioku')->lookup('dashboard templates'));
> }
> EOF
>
> git commit --fixup 7765cbd2
>
> Looks fine to me. But then I try to use it:
The shell will expand variables in the <<EOF section above. You can
fix that by changing it to output <<\EOF instead, which is clearly a
bug in the original script.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH v2] completion: fix issue with process substitution not working on Git for Windows
From: Stefan Naewe @ 2011-10-26 19:13 UTC (permalink / raw)
To: spearce; +Cc: git, gitster, Stefan Naewe
In-Reply-To: <CAJzBP5QYKOf4OUMm4vfVay=b7F_fHJf40JgzDAZZa7p0fxLpyA@mail.gmail.com>
Git for Windows comes with a bash that doesn't support process substitution.
It issues the following error when using git-completion.bash with
GIT_PS1_SHOWUPSTREAM set:
$ export GIT_PS1_SHOWUPSTREAM=1
sh.exe": cannot make pipe for process substitution: Function not implemented
sh.exe": cannot make pipe for process substitution: Function not implemented
sh.exe": <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n '): ambiguous redirect
Replace the process substitution with a 'here string'.
Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
---
contrib/completion/git-completion.bash | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8648a36..0b3d47e 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -110,6 +110,7 @@ __git_ps1_show_upstream ()
local upstream=git legacy="" verbose=""
# get some config options from git-config
+ output="$(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')"
while read key value; do
case "$key" in
bash.showupstream)
@@ -125,7 +126,7 @@ __git_ps1_show_upstream ()
upstream=svn+git # default upstream is SVN if available, else git
;;
esac
- done < <(git config -z --get-regexp '^(svn-remote\..*\.url|bash\.showupstream)$' 2>/dev/null | tr '\0\n' '\n ')
+ done <<< "$output"
# parse configuration values
for option in ${GIT_PS1_SHOWUPSTREAM}; do
--
1.7.7.1
^ permalink raw reply related
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Johannes Schindelin @ 2011-10-26 18:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: msysgit, git
In-Reply-To: <7vvcrb3c69.fsf@alter.siamese.dyndns.org>
Him
On Wed, 26 Oct 2011, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Rather nasty things happen when a mutex is not initialized but locked
> > nevertheless. Now, when we're not running in a threaded manner, the
> > mutex is not initialized, which is correct.
>
> Thanks; I wonder why pack-objects does not have the same issue, though.
Those tests do not fail here, so I did not investigate.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH/WIP 02/11] notes-merge: use opendir/readdir instead of using read_directory()
From: Junio C Hamano @ 2011-10-26 17:37 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CocoAiVx_PeaaX1oRZvmzfj9-z9JLJkE5unSRVtpGkNA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Current read_directory() treats given path separately from contents
> inside the path. If the given path has ".git", it's ok (but it'll stop
> at .git if during tree recursion). The new read_directory() does not
> make this exception, so when note-merge call
> read_directory(".git/NOTES_MERGE_WORKTREE"), read_directory() sees
> ".git" and stops immediately, assuming it's a gitlink.
When read_directory("where/ever") is called, what kind of paths does it
collect? Do the paths the function collects share "where/ever" as their
common prefix? I thought it collects the paths relative to whatever
top-level directory given to the function, so that "where/ever" could be
anything.
Why does it even have to look at the given path in the first place and
make a decision heavier than "can I opendir() and read from it"? In other
words, if you see read_directory("some/thing/.git/more/stuff") and find a
substring ".git/" in there, what "magic" gitlink handling does the code
have to do?
I do not think it matters for _this_ particular case, but I can for
example imagine an alternative implementation of a merge that uses
temporary working tree somewhere other than the main working tree, and one
of the natural "temporary" places such a feature in the future may want to
use is inside .git/ somewhere. If you are planning to close the door by
breaking the behaviour of read_directory(".git/some_where") for such
callers with this series, we need to be aware of it, and that is why I am
not satisfied by your explanation.
^ permalink raw reply
* Re: [PATCH/WIP 03/11] t5403: avoid doing "git add foo/bar" where foo/.git exists
From: Junio C Hamano @ 2011-10-26 17:26 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8CjJnO6rDVTV1tC9rWXP51LHBtUvNsgVWNfwC+HuNQ-6Q@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Now look from "git add" perspective, it does not really care where
> $GIT_DIR is.
> It assumes that $(pwd) is working directory's top. So it
Now you confused me.
Doesn't it use $GIT_DIR to find the index? And it decides that it is at
the top level because it is given GIT_DIR but not GIT_WORKING_TREE which
is how working tree discovery is defined.
> - reads content of current directory, it sees "clone2" as a directory
> - it descends in and see ".git" so "clone2" must be a git link
> - because clone2 is a separate repository (again $GIT_DIR is not
> consulted), "b" should be managed by "clone2"
> - so it stops.
>
> This is the only place I see a submodule (from the first glance) is
> actually top level repository. Yes I guess we can support this, but
> it's just too weird to be widely used in pratice..
Where did you get this idea that submodule is somehow involved in this test?
I do not see there is any submodules involved; the test uses two
repositories 1 and 2 that appear in the working tree of the main
repository test infrastructure created, but otherwise there is no
sub/super relation among these three, and there are many other tests with
"clone" or "mkdir+init" or "init <newdir>" in the main test repository.
The clone2 repository tracks a path without having a corresponding file in
its working tree (i.e. it has "b" but tracks "clone2/b") which I already
is said unusual, but unusual does not mean it is bad or we want to remove
a test that covers the unusual case to let a change that regresses the
case go unnoticed.
>> Running (cd clone2 && git add b) is a _more natural_ thing to do, and it
>> will result in a path "b" added to the clone2 repository, so that the
>> result is more useful if you are going to chdir to the repository and keep
>> working on it. But that does not mean the existing test is incorrect. It
>> does not just pass somehow but the test passes by design.
>>
>> I did not check if later tests look at the contents of commit "new2" to
>> make sure it contains "clone2/b", but if they do this change should break
>> such tests.
>>
>> So I am puzzled by this change; what is this trying to achieve?
So again, what is this change trying to achieve?
^ permalink raw reply
* Re: [PATCH] git grep: be careful to use mutices only when they are initialized
From: Junio C Hamano @ 2011-10-26 17:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: msysgit, git
In-Reply-To: <alpine.DEB.1.00.1110251223500.32316@s15462909.onlinehome-server.info>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Rather nasty things happen when a mutex is not initialized but locked
> nevertheless. Now, when we're not running in a threaded manner, the mutex
> is not initialized, which is correct.
Thanks; I wonder why pack-objects does not have the same issue, though.
^ permalink raw reply
* [PATCH] Fix 'Cloning into' message
From: Richard Hartmann @ 2011-10-26 17:05 UTC (permalink / raw)
To: git; +Cc: Richard Hartmann
Without this patch,
git clone foo .
results in this:
Cloning into ....
done.
With it:
Cloning into '.'...
done.
---
builtin/clone.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 488f48e..efe8b6c 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -577,9 +577,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
if (0 <= option_verbosity) {
if (option_bare)
- printf(_("Cloning into bare repository %s...\n"), dir);
+ printf(_("Cloning into bare repository '%s'...\n"), dir);
else
- printf(_("Cloning into %s...\n"), dir);
+ printf(_("Cloning into '%s'...\n"), dir);
}
init_db(option_template, INIT_DB_QUIET);
write_config(&option_config);
--
1.7.7
^ permalink raw reply related
* Re: FYI: status of svn-fe speed
From: Tay Ray Chuan @ 2011-10-26 16:18 UTC (permalink / raw)
To: David Michael Barr; +Cc: git, Jonathan Nieder, Dmitry Ivankov
In-Reply-To: <CAFfmPPNK+D=g5h7bdYmON++HE5jF_opYxKLobqjOosj--8+9FQ@mail.gmail.com>
On Wed, Oct 26, 2011 at 11:18 PM, David Michael Barr
<davidbarr@google.com> wrote:
> I talked a lot about low-level speed optimisation after the high-level
> optimisation is finished this week. I merged my svn-fe-pu branch with
> jch/next and tested with a 1000 rev dump of patches from git-core.
> When not blocked, svn-fe can process such revs at ~5000 rev/s on my
I had to rub my eyes when I read this - what???
Good job, then!
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [msysGit] [PATCH] git grep: be careful to use mutices only when they are initialized
From: Johannes Schindelin @ 2011-10-26 15:42 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: msysgit, gitster, git
In-Reply-To: <CALUzUxpVWHL8LyqYkYazxSxDr6i=kitACFfVRQsTxQHHYjiOyA@mail.gmail.com>
Hi Tay,
On Wed, 26 Oct 2011, Tay Ray Chuan wrote:
> On Wed, Oct 26, 2011 at 1:25 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> >
> > Rather nasty things happen when a mutex is not initialized but locked
> > nevertheless. Now, when we're not running in a threaded manner, the
> > mutex is not initialized, which is correct. But then we went and used
> > the mutex anyway, which -- at least on Windows -- leads to a hard
> > crash (ordinarily it would be called a segmentation fault, but in
> > Windows speak it is an access violation).
> >
> > This problem was identified by our faithful tests when run in the
> > msysGit environment.
>
> May I ask which test are you talking about specifically?
It is t7810.
> I ask as I'm curious how this is triggered; git-grep works fine for me
> so far (1.7.6.msysgit.0.584.g2cbf)
That did not expose the error. The problem is exposed in msysGit's 'devel'
branch, though.
Ciao,
Johannes
^ permalink raw reply
* FYI: status of svn-fe speed
From: David Michael Barr @ 2011-10-26 15:18 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Dmitry Ivankov
Hi,
I talked a lot about low-level speed optimisation after the high-level
optimisation is finished this week. I merged my svn-fe-pu branch with
jch/next and tested with a 1000 rev dump of patches from git-core.
When not blocked, svn-fe can process such revs at ~5000 rev/s on my
laptop. This is far above what both svnadmin dump and git fast-import
presently achieve and might be deemed excessive. However, this
translates to low latency which is critical for parallelisation.
I had to use a 10 microsecond sampling interval to get an accurate
profile. These numbers have not been normalised but are the complete
set of symbols sampled.
2.2% parse_date_basic
1.5% match_string
1.1% svndump_read
1.1% 0x10000af70 [20.2KB]
0.4% strbuf_grow
0.4% strbuf_fread
0.4% strbuf_add
0.4% reset_rev_ctx
0.4% next_quote_pos
0.4% is_date
0.4% fast_export_data
0.4% end_revision
0.4% buffer_read_line
0.4% buffer_copy_bytes
I still think it ought to be a little faster. ;)
--
David Barr
^ permalink raw reply
* Re: git-fixup-assigner.perl -- automatically decide where to "fixup!"
From: fREW Schmidt @ 2011-10-26 14:37 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <201012140309.59378.trast@student.ethz.ch>
On Mon, Dec 13, 2010 at 8:09 PM, Thomas Rast <trast@student.ethz.ch> wrote:
>
> While cleaning up the 'log -L' series I gathered a large number of
> little fixups, and decided it would be smart if git could
> automatically figure out where to put them.
>
> It works like this:
>
> * Split the diff by hunk. I'm using -U1 here for finer splits, but it
> could be tunable.
>
> * For each hunk, run blame to find out which commit's lines were
> affected.
>
> * Group the hunks by this commit, and output them with a suitable
> command to make a fixup.
>
> My git-fixup is
>
> $ g config alias.fixup
> !sh -c 'r=$1; git commit -m"fixup! $(git log -1 --pretty=%s $r)"' -
>
> so that is "suitable".
>
> You would run it with the changes unstaged in your tree as
>
> ./git-fixup-assigner.perl > fixups
>
> and can then review with 'less fixups', or run 'sh fixups' to commit
> them.
>
> It's certainly not perfect, notably the detection logic should ignore
> context, but it got the job done.
>
> --- 8< ---
> #!/usr/bin/perl
>
> use warnings;
> use strict;
>
> sub parse_hunk_header {
> my ($line) = @_;
> my ($o_ofs, $o_cnt, $n_ofs, $n_cnt) =
> $line =~ /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/;
> $o_cnt = 1 unless defined $o_cnt;
> $n_cnt = 1 unless defined $n_cnt;
> return ($o_ofs, $o_cnt, $n_ofs, $n_cnt);
> }
>
> sub find_commit {
> my ($file, $begin, $end) = @_;
> my $blame;
> open($blame, '-|', 'git', '--no-pager', 'blame', 'HEAD', "-L$begin,$end", $file) or die;
> my %candidate;
> while (<$blame>) {
> $candidate{$1} += 1 if /^([0-9a-f]+)/;
> }
> close $blame or die;
> my @sorted = sort { $candidate{$b} <=> $candidate{$a} } keys %candidate;
> if (1 < scalar @sorted) {
> print STDERR "ambiguous split $file:$begin..$end\n";
> foreach my $c (@sorted) {
> print STDERR "\t$candidate{$c}\t$c\n";
> }
> }
> return $sorted[0];
> }
>
> my $diff;
> open($diff, '-|', 'git', '--no-pager', 'diff', '-U1') or die;
>
> my %by_commit;
> my @cur_hunk = ();
> my $cur_commit;
> my ($filename, $prefilename, $postfilename);
>
> while (<$diff>) {
> if (m{^diff --git ./(.*) ./\1$}) {
> if (@cur_hunk) {
> push @{$by_commit{$cur_commit}{$filename}}, @cur_hunk;
> @cur_hunk = ();
> }
> $filename = $1;
> $prefilename = "./" . $1;
> $postfilename = "./" . $1;
> } elsif (m{^index}) {
> # ignore
> } elsif (m{^new file}) {
> $prefilename = '/dev/null';
> } elsif (m{^delete file}) {
> $postfilename = '/dev/null';
> } elsif (m{^--- $prefilename$}) {
> } elsif (m{^\+\+\+ $postfilename$}) {
> } elsif (m{^@@ }) {
> if (@cur_hunk) {
> push @{$by_commit{$cur_commit}{$filename}}, @cur_hunk;
> @cur_hunk = ();
> }
> push @cur_hunk, $_;
> die "I don't handle this diff" if ($prefilename ne $postfilename);
> my ($o_ofs, $o_cnt, $n_ofs, $n_cnt)
> = parse_hunk_header($_);
> my $o_end = $o_ofs + $o_cnt - 1;
> $cur_commit = find_commit($filename, $o_ofs, $o_end);
> } elsif (m{^[-+ \\]}) {
> push @cur_hunk, $_;
> } else {
> die "unhandled diff line: '$_'";
> }
> }
>
> close $diff or die;
>
> if (@cur_hunk) {
> push @{$by_commit{$cur_commit}{$filename}}, @cur_hunk;
> @cur_hunk = ();
> }
>
> print "#!/bin/sh\n\n";
>
> foreach my $commit (keys %by_commit) {
> print "git apply --cached <<EOF\n";
> foreach my $filename (keys %{$by_commit{$commit}}) {
> print "diff --git a/$filename b/$filename\n";
> print "--- a/$filename\n";
> print "+++ b/$filename\n";
> print @{$by_commit{$commit}{$filename}};
> }
> print "EOF\n\n";
> print "git fixup $commit\n\n";
> }
> --- >8 ---
This is super neat, but I'm having trouble getting it to work.
First, I made one small change:
- print "git fixup $commit\n\n";
+ print "git commit --fixup $commit\n\n";
To try it out I made a very simple change:
$ git diff
diff --git a/App/lib/MyApp/Controller/DashboardTemplates.pm
b/App/lib/MyApp/Controller/DashboardTemplates.pm
index aefdc3c..a53b534 100644
--- a/App/lib/MyApp/Controller/DashboardTemplates.pm
+++ b/App/lib/MyApp/Controller/DashboardTemplates.pm
@@ -13,7 +13,7 @@ cat_has $_ => ( is => 'rw' ) for qw(set);
sub base : Chained('/') PathPart('dashboard_templates') CaptureArgs(0) {
my ($self, $c) = @_;
- $self->set($c,
$c->model('DB')->schema->kiokudb_handle->lookup('dashboard
templates'));
+ $self->set($c, $c->model('Kioku')->lookup('dashboard templates'));
}
my $renderer = sub {
Then I tried it out
$ git fixup-assigner.pl > fixups && less fixups
#!/bin/sh
git apply --cached <<EOF
diff --git a/App/lib/MyApp/Controller/DashboardTemplates.pm
b/App/lib/MyApp/Controller/DashboardTemplates.pm
--- a/App/lib/MyApp/Controller/DashboardTemplates.pm
+++ b/App/lib/MyApp/Controller/DashboardTemplates.pm
@@ -15,3 +15,3 @@ sub base : Chained('/')
PathPart('dashboard_templates') CaptureArgs(0) {
my ($self, $c) = @_;
- $self->set($c,
$c->model('DB')->schema->kiokudb_handle->lookup('dashboard
templates'));
+ $self->set($c, $c->model('Kioku')->lookup('dashboard templates'));
}
EOF
git commit --fixup 7765cbd2
Looks fine to me. But then I try to use it:
$ git checkout . && sh fixups
error: patch failed: App/lib/MyApp/Controller/DashboardTemplates.pm:15
error: App/lib/MyApp/Controller/DashboardTemplates.pm: patch does not apply
Any ideas what I'm doing wrong?
--
fREW Schmidt
http://blog.afoolishmanifesto.com
^ permalink raw reply
* Re: [msysGit] Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
From: Erik Faye-Lund @ 2011-10-26 13:16 UTC (permalink / raw)
To: Kyle Moffett; +Cc: Johannes Sixt, msysgit, git, johannes.schindelin
In-Reply-To: <CAGZ=bqJ7k5h_-62M3y-Jype4a7mOTe+FxeU13JreGj0mOnSRSg@mail.gmail.com>
On Wed, Oct 26, 2011 at 5:44 AM, Kyle Moffett <kyle@moffetthome.net> wrote:
> On Tue, Oct 25, 2011 at 16:51, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> On Tue, Oct 25, 2011 at 10:07 PM, Johannes Sixt <j6t@kdbg.org> wrote:
>>> Am 25.10.2011 17:42, schrieb Erik Faye-Lund:
>>>> On Tue, Oct 25, 2011 at 5:28 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>>>> Am 10/25/2011 16:55, schrieb Erik Faye-Lund:
>>>>>> +int pthread_mutex_lock(pthread_mutex_t *mutex)
>>>>>> +{
>>>>>> + if (mutex->autoinit) {
>>>>>> + if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) != -1) {
>>>>>> + pthread_mutex_init(mutex, NULL);
>>>>>> + mutex->autoinit = 0;
>>>>>> + } else
>>>>>> + while (mutex->autoinit != 0)
>>>>>> + ; /* wait for other thread */
>>>>>> + }
>>>>>
>>>>> The double-checked locking idiom. Very suspicious. Can you explain why it
>>>>> works in this case? Why are no Interlocked functions needed for the other
>>>>> accesses of autoinit? ("It is volatile" is the wrong answer to this last
>>>>> question, BTW.)
>>>>
>>>> I agree that it should look a bit suspicious; I'm generally skeptical
>>>> whenever I see 'volatile' in threading-code myself. But I think it's
>>>> the right answer in this case. "volatile" means that the compiler
>>>> cannot optimize away accesses, which is sufficient in this case.
>>>
>>> No, it is not, and it took me a train ride to see what's wrong. It has
>>> nothing to do with autoinit, but with all the other memory locations
>>> that are written. See here, with pthread_mutex_init() inlined:
>>>
>>> if (mutex->autoinit) {
>>>
>>> Assume two threads enter this block.
>>>
>>> if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) != -1) {
>>>
>>> Only one thread, A, say on CPU A, will enter this block.
>>>
>>> InitializeCriticalSection(&mutex->cs);
>>>
>>> Thread A writes some values. Note that there are no memory barriers
>>> involved here. Not that I know of or that they would be documented.
>>>
>>> mutex->autoinit = 0;
>>>
>>> And it writes another one. Thread A continues below to contend for the
>>> mutex it just initialized.
>>>
>>> } else
>>>
>>> Meanwhile, thread B, say on CPU B, spins in this loop:
>>>
>>> while (mutex->autoinit != 0)
>>> ; /* wait for other thread */
>>>
>>> When thread B arrives here, it sees the value of autoinit that thread A
>>> has written above.
>>>
>>> HOWEVER, when it continues, there is NO [*] guarantee that it will also
>>> see the values that InitializeCriticalSection() has written, because
>>> there were no memory barriers involved. When it continues, there is a
>>> chance that it calls EnterCriticalSection() with uninitialized values!
>>>
>>
>> Thanks for pointing this out, I completely forgot about write re-ordering.
>>
>> This is indeed a problem. So, shouldn't replacing "mutex->autoinit =
>> 0;" with "InterlockedExchange(&mutex->autoinit, 0)" solve the problem?
>> InterlockedExchange generates a full memory barrier:
>> http://msdn.microsoft.com/en-us/library/windows/desktop/ms683590(v=vs.85).aspx
>
> No, I'm afraid that won't solve the issue (at least in GCC, not sure about MSVC)
>
> A write barrier in one thread is only effective if it is paired with a
> read barrier in the other thread.
>
> Since there's no read barrier in the "while(mutex->autoinit != 0)",
> you don't have any guaranteed ordering.
>
> I guess if MSVC assumes that volatile reads imply barriers then it might work...
OK, so I should probably do something like this instead?
while (InterlockedCompareExchange(&mutex->autoinit, 0, 0) != 0)
; /* wait for other thread */
I really appreciate getting some extra eyes on this, thanks.
Concurrent programming is not my strong-suit (as this exercise has
shown) ;)
^ permalink raw reply
* Re: [msysGit] Re: [PATCH/RFC] mingw: implement PTHREAD_MUTEX_INITIALIZER
From: Erik Faye-Lund @ 2011-10-26 13:08 UTC (permalink / raw)
To: msysgit; +Cc: git, johannes.schindelin, j.sixt
In-Reply-To: <16520370.401.1319598319120.JavaMail.geo-discussion-forums@prms22>
On Wed, Oct 26, 2011 at 5:05 AM, Atsushi Nakagawa <atnak@chejz.com> wrote:
> On Oct 25, 11:55 pm, Erik Faye-Lund <kusmab...@gmail.com> wrote:
>> [...]
>> +int pthread_mutex_init(pthread_mutex_t *mutex, const pthread_mutexattr_t
>> *attr)
>> +{
>> + InitializeCriticalSection(&mutex->cs);
>> + mutex->autoinit = 0;
>> + return 0;
>> +}
>> +
>> +int pthread_mutex_lock(pthread_mutex_t *mutex)
>> +{
>> + if (mutex->autoinit) {
>> + if (InterlockedCompareExchange(&mutex->autoinit, -1, 1) !=
>> -1) {
> I'm making the assumption that mutex->autoinit starts off as 1 before things
> get multi-threaded..
> I've only looked at what's in the patch so I could be missing vital
> context.. Anyways, is there a reason why you made this
> "InterlockedCompareExchange(..., -1, 1) != -1" and not
> "InterlockedCompareExchange(..., -1, 1) == 1"?
No, not really.
> It looks to me the former adds a race condition after "if (mutex->autoinit)
> {". e.g. A second thread could reinitialize mutex->cs after the first
> thread has already entered EnterCriticalSection(...).
You are indeed correct, thanks for spotting :)
^ permalink raw reply
* Re: git merge successfully however this is still issue from logical perspective
From: Thomas Rast @ 2011-10-26 12:39 UTC (permalink / raw)
To: Lynn Lin; +Cc: git
In-Reply-To: <CAPgpnMR6_pRxMSLcdS=M4Cfj=dqk6KTXr3VGhk3LrnPHyv2waA@mail.gmail.com>
Lynn Lin wrote:
> On Wed, Oct 26, 2011 at 8:27 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> > Lynn Lin wrote:
> >> Hi all,
> >> Do this case can happen? When I do merge from one branch to master
> >> branch,merge successfully however from code logical perspective it
> >> fails and It cause code compile (our project is using C++ language)
> >
> > Sure. The easiest example is when one side of a merge renames foo()
> > to bar(), while the other side introduces another call to foo() in an
> > unrelated part of the code.
> Does this mean we shouldn't trust merge result ?
Depends what you mean by "trust".
If you expected a merge to understand what the changes on each side
were about, and then patch the code accordingly, no it can't do
that.[*] You are in no way guaranteed that the result of a merge is
correct or even compiles even if both sides of the merge were.
But after all git-merge is just a tool doing a well-defined operation:
textually merging the changes from both sides. AFAIK it could so far
always be trusted with doing *that* correctly.
[*] darcs can go half of the way by having more expressive
"changesets", but this can also fail. I think the general problem is
AI-complete.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: git merge successfully however this is still issue from logical perspective
From: Lynn Lin @ 2011-10-26 12:29 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <201110261427.03585.trast@student.ethz.ch>
On Wed, Oct 26, 2011 at 8:27 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> Lynn Lin wrote:
>> Hi all,
>> Do this case can happen? When I do merge from one branch to master
>> branch,merge successfully however from code logical perspective it
>> fails and It cause code compile (our project is using C++ language)
>
> Sure. The easiest example is when one side of a merge renames foo()
> to bar(), while the other side introduces another call to foo() in an
> unrelated part of the code.
Does this mean we shouldn't trust merge result ?
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox