Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()
Karthik Nayak karthik@gmail.com writes:
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int
unset)
+{
+ struct ref_filter *rf = opt-value;
+ unsigned char sha1[20];
+
+ rf-merge = opt-long_name[0] == 'n'
+ ? REF_FILTER_MERGED_OMIT
+ : REF_FILTER_MERGED_INCLUDE;
I would use starts_with(no-, opt-long_name) instead. I had a hard
time understanding why the letter 'n' was special while the
starts_with() version is self-explanatory.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 10/11] for-each-ref: introduce filter_refs()
Karthik Nayak karthik@gmail.com writes:
+/*
+ * API for filtering a set of refs. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct
ref_filter_cbdata *data)
+{
+ return for_each_ref_fn(ref_filter_handler, data);
+}
I do not think it is such a good idea to allow API callers to
specify for-each-ref-fn directly. See my message in an earlier
review.
I also think ref_filter_cbdata is an implementation detail of
filter_refs and may not have to be exposed to the API callers.
It probably is more sensible for them to pass
- an array of refs to receive filtered results (your ref_array thing)
- the criteria to use when filtering (your ref_filter thing)
as two separate parameters to this function, together with other
parameters that lets you (meaning the implementation of filter_refs())
to decide which for-each-ref iterator to call, e.g. do you want to
use raw iteration? do you want to iterate only over refs/heads? etc.
In other words, the caller of this API should not have to know that
you (meaning the implementation of filter_refs()) are internally
using for_each_ref() API.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option
Karthik Nayak karthik@gmail.com writes:
In 'tag -l' we have '--points-at' option which lets users
list only tags which point to a particular commit, Implement
s/,/./ ?
this option in 'ref-filter.{c,h}' so that the other commands
s/the//
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
GNU diff and git diff - difference on myers algorithm?
Based on a cursory review of the git code I get the impression that GNU diff and git 'diff' do not share any code for the possible diff algorithms. I'm in particularly curious more about the default myers algorithm. I can take time to do a precise code review of the algorithms used on both GNU diff and git but if someone can already vet for any differences that'd be appreciated as it would save time. Luis -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 5/9] ref-filter: implement '--merged' and '--no-merged' options
Karthik Nayak karthik@gmail.com writes:
+ if(filter-merge_commit) {
space after if.
@@ -938,7 +991,13 @@ void ref_array_clear(struct ref_array *array)
*/
int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct
ref_filter_cbdata *data)
{
- return for_each_ref_fn(ref_filter_handler, data);
+ int ret;
+
+ ret = for_each_ref_fn(ref_filter_handler, data);
+ if (data-filter.merge_commit)
+ do_merge_filter(data);
Reading this, I first wondered why you did not do the merge_filter as
part of ref_filter_handler. It seems weird to fill-in an array and then
re-filter it. I think it would make sense to add a few comments, like
/* Simple per-ref filtering */
ret = for_each_ref_fn(ref_filter_handler, data);
/* Filters that need revision walking */
if (data-filter.merge_commit)
...
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option
On 06/08/2015 11:01 PM, Matthieu Moy wrote:
Karthik Nayak karthik@gmail.com writes:
+/*
+ * Given a ref (sha1, refname) see if it points to a one of the sha1s
+ * in a sha1_array.
+ */
+static int match_points_at(struct sha1_array *points_at, const unsigned char
*sha1,
+ const char *refname)
+{
+ struct object *obj;
+
+ if (!points_at || !points_at-nr)
+ return 1;
+
+ if (sha1_array_lookup(points_at, sha1) = 0)
+ return 1;
+
+ obj = parse_object_or_die(sha1, refname);
+ if (obj-type == OBJ_TAG
+ sha1_array_lookup(points_at, ((struct tag *)obj)-tagged-sha1) =
0)
+ return 1;
+
+ return 0;
+}
There's a similar function in builtin/tag.c that you are not removing.
You should justify why you are doing this code duplication in the commit
message (or not do code duplication). It might make sense to add a
comment next to match_points_at in tag.c saying stg like this is
duplicated from ..., will be removed later.
Ok will add this :)
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/9] add options to ref-filter
Karthik Nayak karthik@gmail.com writes: This is a follow up series to the one posted here http://thread.gmane.org/gmane.comp.version-control.git/270922 This patch series adds '--ponints-at', '--merged', '--no-merged' and '--contain' options to 'ref-filter' and uses these options in for-each-ref'. The structure of the series looked sensible (I stopped at around 7/9, though, for now); even though a few style violations were somewhat irritating, overall it was a pleasant read. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/5] am: teach StGit patch parser how to read from stdin
Paul Tan pyoka...@gmail.com writes:
git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is - or there are no files listed on the
command-line.
To be consistent with this behavior, teach the StGit patch parser to
read from stdin if the filename is - or no files are listed on the
command-line.
Hmm, doesn't
perl -ne 'processing for each line'
with or without a BEGIN {} block, read from the standard input (if
no filename is given) or the given file (if given), and more
importantly, doesn't it treat a lone - as STDIN anyway?
That is, wouldn't it make more sense to do something like:
test $# != 0 || set -- -
for stgit
do
...
@@PERL@@ -ne 'BEGIN { $subject = 0 }
...
' $stgit $dotest/$msgnum || clean_abort
done
Same for patch 5/5.
Other than that, the entire series looked great from a cursory read.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-rebase--interactive.sh: add config option for custom
Mike Rappazzo rappa...@gmail.com writes:
On Mon, Jun 8, 2015 at 11:28 AM, Junio C Hamano gits...@pobox.com wrote:
This is optional, but I still wonder why the command line cannot be
more like this, though:
format=$(git config --get rebase.insnFormat)
git log --format=%H ${format-%s} --reverse --right-only
--topo-order \
$revisions ${restrict_revision+^$restrict_revision} |
while read -r sha1 junk
do
...
That way we can optimize one sed process away.
If this is a good idea, it needs to be a separate follow-up patch
that changes %m filtered by sed to use --right-only. I do not
think such a change breaks anything, but I do not deal with complex
histories myself, so...
As far as I can tell, the rev-list will return multiple lines when not
using 'oneline'. The 'sed -n' will join the lines back together.
There is no joining going on.
To rev-list, a custom --pretty/--format is a signal to trigger its
verbose mode, and it shows a commit object-name line and then
the line in the format specified, e.g.
$ git rev-list --pretty='%m%H %(35,trunc)%s' --right-only --reverse ...2024d3
commit 1e9676ec0a771de06abca3009eb4bdc5a4ae3312
1e9676ec0a771de06abca3009eb4bdc5a4ae3312 lockfile: replace random() by ran..
commit 2024d3176536fd437b4c0a744161e96bc150a24e
2024d3176536fd437b4c0a744161e96bc150a24e help.c: wrap wait-only poll() inv..
...
Because of that, format=%m | sed -n s///p would be one way to
make sure that all lines we care about are prefixed by '' so that
we can pick them while discarding anything else. So you do need
filtering unless switch to log, even if you used --right-only.
That is why I didn't use rev-list in the message you are
responding to.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 01/11] for-each-ref: extract helper functions out of grab_single_ref()
Karthik Nayak karthik@gmail.com writes: + * Given a refname, return 1 if the refname matches with one of the patterns You can match refname with pattern. But refname matches the pattern without with, I think. I am not a native, though. + * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master' + * and so on, else return 0. Supports use of wild characters. s/wild/wildcard/. Return 1 if the refname matches with one of the patterns, otherwise 0. The patterns can be literal prefix (e.g. a refname refs/heads/master matches a pattern refs/heads/) or a wildcard (e.g. the same ref matches refs/heads/m*, too). perhaps? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 09/11] ref-filter: move code from 'for-each-ref'
Karthik Nayak karthik@gmail.com writes: On 06/08/2015 10:38 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -1129,7 +56,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) memset(ref_cbdata, 0, sizeof(ref_cbdata)); ref_cbdata.filter.name_patterns = argv; -for_each_rawref(ref_filter_handler, ref_cbdata); +filter_refs(for_each_rawref, ref_cbdata); This seems unrelated from the rest of the patch. And you haven't introduced filter_refs yet! That definitely is, this happened after fixing merges I suppose. Ignore it for now, I'll fix it. I am not sure which one of two copies of 09/11 is being discussed, but I'll stop here for now. Most of the patches up to 08/11 looked sensible, if the renames were a bit too noisy. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Suggestion: group files in GIT
On Mon, Jun 8, 2015 at 10:50 AM, Konrád Lőrinczi klorin...@gmail.com wrote: I would like to group some files, so I can list group files together, list group changes together, filter by group for staging, also order by group. It seems, there is no such feature in GIT I would need, so I send it as suggestion. We can call this feature as Group files or Label files (labeling is used in Gmail, so this may be also a naming alternative). Example file list I would like to group together into [group1]: theme/header.php theme/footer.php theme/body.php lib/theme.php Can't you use a shell variable like: group1=theme/header.php theme/footer.php theme/body.php lib/theme.php ? They are in different directories, but mostly belongs together, so if I group them, then I can work easier with them. - I could select a file group for staging, so only the changes in the group would be added to stage. git add $group1 Changed files in the group: [group1]/theme/header.php [group1]/lib/theme.php - I could list files filtered by a group. Files filtered by [group1]: [group1]/theme/header.php [group1]/theme/footer.php [group1]/theme/body.php [group1]/lib/theme.php ls -l $group1 - I could order file list to list group files first, then directory files. [group1]/theme/header.php [group1]/theme/footer.php [group1]/theme/body.php [group1]/lib/theme.php other/files.php I am not sure I see what you want to do with that. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 5/5] send-email: refactor address list process
Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: Simplify code by creating a function to transform list of email lists (comma separated, with aliases ...) into a simple list of valid email addresses. I would have found the series easier to read if this refactoring came earlier (and then PATCH 2/5 would fix the bug as a positive side effect of the refactoring). I agree that doing 5/5 sooner would make 4/5 a lot clearer. Introducing the helper of 5/5 before 2/5 happens, and then replacing two calls to validate-address-list with process-address-list would hide the nature of the change, i.e. fixing a bug, so it is better to see it done before the refactoring of 5/5, provided if it is indeed a bug that these were not expanded. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option
On 06/08/2015 11:05 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: Add the '--points-at' option provided by 'ref-filter'. The option lets the user to pick only refs which point to a particular commit. Add Documentation for the same. ... but no test? No haven't written tests, this was just to ensure if the way this is moving forth is correct. -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()
Karthik Nayak karthik@gmail.com writes:
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int
unset)
+{
+ struct ref_filter *rf = opt-value;
+ unsigned char sha1[20];
+
+ rf-merge = opt-long_name[0] == 'n'
+ ? REF_FILTER_MERGED_OMIT
+ : REF_FILTER_MERGED_INCLUDE;
+
+ if (!arg)
+ arg = HEAD;
+ if (get_sha1(arg, sha1))
+ die(_(malformed object name %s), arg);
+
+ rf-merge_commit = lookup_commit_reference_gently(sha1, 0);
+ if (!rf-merge_commit)
+ return opterror(opt, must point to a commit, 0);
+
+ return 0;
+}
Again, this smells too specific to live as a part of parse-options
infrastructure. If we want to have two helper callbacks, one that
gives the results in an sha1-array (because there is no guarantee
that you want only commits) and in a commit-list, I am fine with
having parse_opt_object_name() and parse_opt_with_commit(). Perhaps
rename the latter which was named too specifically to something more
sensible (e.g. parse_opt_commit_object_name()) and use it from the
caller you wanted to use parse_opt_merge_filter()? The caller, if
it is not prepared to see more than one commits specified, may have
to check if (!list || !list-next) { die(I want one and only one) }
or something, though.
Having it in ref-filter.h as parse_opt_merge_filter() is fine,
though. After all, you would be sharing it with for-each-ref,
branch and tag and nobody else anyway.
diff --git a/parse-options.h b/parse-options.h
index 3ae16a1..7bcf0f3 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -221,6 +221,7 @@ extern int parse_opt_expiry_date_cb(const struct option
*, const char *, int);
extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
extern int parse_opt_points_at(const struct option *, const char *, int);
+extern int parse_opt_merge_filter(const struct option *, const char *, int);
extern int parse_opt_with_commit(const struct option *, const char *, int);
extern int parse_opt_tertiary(const struct option *, const char *, int);
extern int parse_opt_string_list(const struct option *, const char *, int);
@@ -243,5 +244,15 @@ extern int parse_opt_noop_cb(const struct option *,
const char *, int);
OPT_COLOR_FLAG(0, color, (var), (h))
#define OPT_COLUMN(s, l, v, h) \
{ OPTION_CALLBACK, (s), (l), (v), N_(style), (h), PARSE_OPT_OPTARG,
parseopt_column_callback }
+#define OPT_NO_MERGED(filter, h) \
+ { OPTION_CALLBACK, 0, no-merged, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+ parse_opt_merge_filter, (intptr_t) HEAD \
+ }
+#define OPT_MERGED(filter, h) \
+ { OPTION_CALLBACK, 0, merged, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG, \
+ parse_opt_merge_filter, (intptr_t) HEAD \
+ }
Likewise.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 7/9] parse-options.h: add macros for '--contains' option
Karthik Nayak karthik@gmail.com writes:
+#define OPT_CONTAINS(filter, h) \
+ { OPTION_CALLBACK, 0, contains, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT, \
+ parse_opt_with_commit, (intptr_t) HEAD \
+ }
+#define OPT_WITH(filter, h) \
+ { OPTION_CALLBACK, 0, with, (filter), N_(commit), (h), \
+ PARSE_OPT_LASTARG_DEFAULT, \
+ parse_opt_with_commit, (intptr_t) HEAD \
+ }
The redundancy bothers me. Can't we do a bit better than that,
perhaps like this?
#define _OPT_CONTAINS_OR_WITH(name, variable, help) \
{ OPTION_CALLBACK, 0, name, (variable), N_(commit), (help), \
PARSE_OPT_LASTARG_DEFAULT, \
parse_opt_with_commit, (intptr_t) HEAD \
}
#define OPT_CONTAINS(v, h) _OPT_CONTAINS_OR_WITH(contains, v, h)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/9] for-each-ref: add '--merged' and '--no-merged' options
On 06/08/2015 11:23 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: Add the '--merged' and '--no-merged' options provided by 'ref-filter'. The '--merged' option lets the user to only list refs merged into the named commit. The '--no-merged' option lets the user to only list refs not merged into the named commit. Add documentation for the same. This also requires some tests. The algorithmic part will be validated by 'git tag --merged' using this library, but you need to check the codepath from the command-line to the lib in for-each-ref. @@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_CALLBACK(0, points-at, ref_cbdata.filter.points_at, N_(object), N_(print only tags of the object), parse_opt_points_at), + OPT_MERGED(ref_cbdata.filter, N_(print only merged refs)), + OPT_NO_MERGED(ref_cbdata.filter, N_(print only not merged refs)), I'd spell that only refs that are not merged. Like I mentioned, no tests were written. Will change that. -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 5/9] ref-filter: implement '--merged' and '--no-merged' options
On 06/08/2015 11:21 PM, Matthieu Moy wrote:
Karthik Nayak karthik@gmail.com writes:
+ if(filter-merge_commit) {
space after if.
@@ -938,7 +991,13 @@ void ref_array_clear(struct ref_array *array)
*/
int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct
ref_filter_cbdata *data)
{
- return for_each_ref_fn(ref_filter_handler, data);
+ int ret;
+
+ ret = for_each_ref_fn(ref_filter_handler, data);
+ if (data-filter.merge_commit)
+ do_merge_filter(data);
Reading this, I first wondered why you did not do the merge_filter as
part of ref_filter_handler. It seems weird to fill-in an array and then
re-filter it. I think it would make sense to add a few comments, like
/* Simple per-ref filtering */
ret = for_each_ref_fn(ref_filter_handler, data);
/* Filters that need revision walking */
if (data-filter.merge_commit)
...
Thanks! will add the space and comments as suggested.
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 1/9] tag: libify parse_opt_points_at()
Karthik Nayak karthik@gmail.com writes:
diff --git a/parse-options-cb.c b/parse-options-cb.c
index be8c413..a4d294d 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -4,6 +4,7 @@
#include commit.h
#include color.h
#include string-list.h
+#include sha1-array.h
...
+int parse_opt_points_at(const struct option *opt, const char *arg, int unset)
+{
+ unsigned char sha1[20];
+
+ if (unset) {
+ sha1_array_clear(opt-value);
+ return 0;
+ }
+ if (!arg)
+ return -1;
+ if (get_sha1(arg, sha1))
+ return error(_(malformed object name '%s'), arg);
+ sha1_array_append(opt-value, sha1);
+ return 0;
+}
+
This feels way too specialized to live as part of parse_options
infrastructure.
The existing caller(s) may want to use this callback for parsing
points-at option they have, but is that the only plausible use of
this callback? It looks to be usable by any future caller that
wants to take and accumulate any object names into an sha1-array, so
perhaps rename it to be a bit more generic to represent its nature
better?
parse_opt_object_name()
or something?
I also wonder if we can (and want to) refactor the users of
with-commit callback. Have them use this to obtain an sha1-array
and then convert what they received into a commit-list themselves.
int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
{
int *target = opt-value;
diff --git a/parse-options.h b/parse-options.h
index c71e9da..3ae16a1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -220,6 +220,7 @@ extern int parse_opt_approxidate_cb(const struct option
*, const char *, int);
extern int parse_opt_expiry_date_cb(const struct option *, const char *,
int);
extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
+extern int parse_opt_points_at(const struct option *, const char *, int);
extern int parse_opt_with_commit(const struct option *, const char *, int);
extern int parse_opt_tertiary(const struct option *, const char *, int);
extern int parse_opt_string_list(const struct option *, const char *, int);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 4/9] parse-options: add parse_opt_merge_filter()
On 06/08/2015 11:28 PM, Matthieu Moy wrote:
Karthik Nayak karthik@gmail.com writes:
+int parse_opt_merge_filter(const struct option *opt, const char *arg, int
unset)
+{
+ struct ref_filter *rf = opt-value;
+ unsigned char sha1[20];
+
+ rf-merge = opt-long_name[0] == 'n'
+ ? REF_FILTER_MERGED_OMIT
+ : REF_FILTER_MERGED_INCLUDE;
I would use starts_with(no-, opt-long_name) instead. I had a hard
time understanding why the letter 'n' was special while the
starts_with() version is self-explanatory.
Can do that also :)
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option
On 06/08/2015 11:30 PM, Matthieu Moy wrote:
Karthik Nayak karthik@gmail.com writes:
In 'tag -l' we have '--points-at' option which lets users
list only tags which point to a particular commit, Implement
s/,/./ ?
this option in 'ref-filter.{c,h}' so that the other commands
s/the//
Noted! thanks
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH [git/contrib] Avoid failing to create ${__git_tcsh_completion_script} when 'set noclobber' is in effect (af7333c)
Please use a subject that is shorter and looks more like others on this list.
On Sun, Jun 7, 2015 at 9:54 PM, Ariel Faigon github.2...@yendor.com wrote:
Junio,
This is my 1st time doing this, sorry.
I hope this satisfies the git Sign Off procedure.
The above lines should not be there, otherwise they will be in the
commit message and will not be useful there.
Problem Description:
Please loose the above header.
tcsh users who happen to have 'set noclobber' elsewhere in their ~/.tcshrc or
~/.cshrc startup files get a 'File exist' error,
When do they get that error?
and the tcsh completion file doesn't get generated/updated. Adding a `!` in
the redirect works correctly for both clobber and noclobber users.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
You don't need to copy the Developer's Certificate of Origin in your
patch even if it's the first time. Your signed-off-by below is enough.
Signed-off-by: Ariel Faigon github.2...@yendor.com
git patch follows.
Please put nothing after your signed-off-by and before the three dashes below.
---
Here, just after the three dashes, is the right place to put personnal
comments and stuff that should not go into the commit message.
contrib/completion/git-completion.tcsh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
index 6104a42..4a790d8 100644
--- a/contrib/completion/git-completion.tcsh
+++ b/contrib/completion/git-completion.tcsh
@@ -41,7 +41,7 @@ if ( ! -e ${__git_tcsh_completion_original_script} ) then
exit
endif
-cat EOF ${__git_tcsh_completion_script}
+cat EOF ! ${__git_tcsh_completion_script}
#!bash
#
# This script is GENERATED and will be overwritten automatically.
Thanks,
Christian.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 2/9] ref-filter: implement '--points-at' option
Karthik Nayak karthik@gmail.com writes:
+/*
+ * Given a ref (sha1, refname) see if it points to a one of the sha1s
+ * in a sha1_array.
+ */
+static int match_points_at(struct sha1_array *points_at, const unsigned char
*sha1,
+const char *refname)
+{
+ struct object *obj;
+
+ if (!points_at || !points_at-nr)
+ return 1;
+
+ if (sha1_array_lookup(points_at, sha1) = 0)
+ return 1;
+
+ obj = parse_object_or_die(sha1, refname);
+ if (obj-type == OBJ_TAG
+ sha1_array_lookup(points_at, ((struct tag *)obj)-tagged-sha1) =
0)
+ return 1;
+
+ return 0;
+}
There's a similar function in builtin/tag.c that you are not removing.
You should justify why you are doing this code duplication in the commit
message (or not do code duplication). It might make sense to add a
comment next to match_points_at in tag.c saying stg like this is
duplicated from ..., will be removed later.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 01/11] for-each-ref: extract helper functions out of grab_single_ref()
On 06/08/2015 11:32 PM, Junio C Hamano wrote: Karthik Nayak karthik@gmail.com writes: + * Given a refname, return 1 if the refname matches with one of the patterns You can match refname with pattern. But refname matches the pattern without with, I think. I am not a native, though. + * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master' + * and so on, else return 0. Supports use of wild characters. s/wild/wildcard/. Return 1 if the refname matches with one of the patterns, otherwise 0. The patterns can be literal prefix (e.g. a refname refs/heads/master matches a pattern refs/heads/) or a wildcard (e.g. the same ref matches refs/heads/m*, too). perhaps? Sounds a lot better, thanks :) -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 10/11] for-each-ref: introduce filter_refs()
On 06/08/2015 11:45 PM, Junio C Hamano wrote:
Karthik Nayak karthik@gmail.com writes:
+/*
+ * API for filtering a set of refs. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct
ref_filter_cbdata *data)
+{
+ return for_each_ref_fn(ref_filter_handler, data);
+}
I do not think it is such a good idea to allow API callers to
specify for-each-ref-fn directly. See my message in an earlier
review.
I did read your previous message. I misunderstood some things.
I also think ref_filter_cbdata is an implementation detail of
filter_refs and may not have to be exposed to the API callers.
It probably is more sensible for them to pass
- an array of refs to receive filtered results (your ref_array thing)
- the criteria to use when filtering (your ref_filter thing)
This could be done.
as two separate parameters to this function, together with other
parameters that lets you (meaning the implementation of filter_refs())
to decide which for-each-ref iterator to call, e.g. do you want to
use raw iteration? do you want to iterate only over refs/heads? etc.
In other words, the caller of this API should not have to know that
you (meaning the implementation of filter_refs()) are internally
using for_each_ref() API.
I'm a little confused about this, how exactly do you propose we go about
doing something like this? I mean, usually the user of the API
knows what exactly they want, like in tag.c, branch.c and for-each-ref.c
But I'm not sure what you mean by parameters that lets you (meaning the
implementation of filter_refs()) to decide which for-each-ref iterator
to call. A small example maybe? Thanks!
--
Regards,
Karthik
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option
Karthik Nayak karthik@gmail.com writes:
Add the '--points-at' option provided by 'ref-filter'. The
option lets the user to pick only refs which point to a particular
commit.
Add Documentation for the same.
Based-on-patch-by: Jeff King p...@peff.net
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
Documentation/git-for-each-ref.txt | 3 +++
builtin/for-each-ref.c | 6 +-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index 7f8d9a5..e9f6a8a 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -10,6 +10,7 @@ SYNOPSIS
[verse]
'git for-each-ref' [--count=count] [--shell|--perl|--python|--tcl]
[(--sort=key)...] [--format=format] [pattern...]
+[--points-at object]
DESCRIPTION
---
@@ -62,6 +63,8 @@ OPTIONS
the specified host language. This is meant to produce
a scriptlet that can directly be `eval`ed.
+--points-at object::
+ Only list tags of the given object.
Is this intended? I would have expected if I did
git for-each-ref --points-at master
I would get refs/heads/master and any other refs that exactly points
at that commit.
FIELD NAMES
---
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 4d2d024..b9d180a 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -7,6 +7,7 @@
static char const * const for_each_ref_usage[] = {
N_(git for-each-ref [options] [pattern]),
+ N_(git for-each-ref [--points-at object]),
NULL
};
@@ -17,6 +18,7 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
struct ref_sorting *sorting = NULL, **sorting_tail = sorting;
int maxcount = 0, quote_style = 0;
struct ref_filter_cbdata ref_cbdata;
+ memset(ref_cbdata, 0, sizeof(ref_cbdata));
struct option opts[] = {
OPT_BIT('s', shell, quote_style,
@@ -33,6 +35,9 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
OPT_STRING( 0 , format, format, N_(format), N_(format to
use for the output)),
OPT_CALLBACK(0 , sort, sorting_tail, N_(key),
N_(field name to sort on),
parse_opt_ref_sorting),
+ OPT_CALLBACK(0, points-at, ref_cbdata.filter.points_at,
+ N_(object), N_(print only tags of the object),
+ parse_opt_points_at),
OPT_END(),
};
@@ -54,7 +59,6 @@ int cmd_for_each_ref(int argc, const char **argv, const
char *prefix)
/* for warn_ambiguous_refs */
git_config(git_default_config, NULL);
- memset(ref_cbdata, 0, sizeof(ref_cbdata));
I cannot quite see how this change relates to the addition of the
new option.
ref_cbdata.filter.name_patterns = argv;
filter_refs(for_each_rawref, ref_cbdata);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 5/5] send-email: refactor address list process
Matthieu Moy matthieu@grenoble-inp.fr writes: To me, the series is ready now, and I don't think re-rolling it would be a good time investment. Plus, I spent time reviewing this series and with my proposal I'd need to review a relatively different one. Ok, thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
Michael Haggerty mhag...@alum.mit.edu writes: Thanks for all the comments. Taking them into consideration, I suggest changing the last commit message to ... Since the only comments were about this one commit message,... Yeah, this round looked good otherwise. Will amend in-place. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/9] for-each-ref: add '--points-at' option
Karthik Nayak karthik@gmail.com writes: Add the '--points-at' option provided by 'ref-filter'. The option lets the user to pick only refs which point to a particular commit. Add Documentation for the same. ... but no test? -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 6/9] for-each-ref: add '--merged' and '--no-merged' options
Karthik Nayak karthik@gmail.com writes: Add the '--merged' and '--no-merged' options provided by 'ref-filter'. The '--merged' option lets the user to only list refs merged into the named commit. The '--no-merged' option lets the user to only list refs not merged into the named commit. Add documentation for the same. This also requires some tests. The algorithmic part will be validated by 'git tag --merged' using this library, but you need to check the codepath from the command-line to the lib in for-each-ref. @@ -38,6 +39,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) OPT_CALLBACK(0, points-at, ref_cbdata.filter.points_at, N_(object), N_(print only tags of the object), parse_opt_points_at), + OPT_MERGED(ref_cbdata.filter, N_(print only merged refs)), + OPT_NO_MERGED(ref_cbdata.filter, N_(print only not merged refs)), I'd spell that only refs that are not merged. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] bisect: replace hardcoded bad|good by variables
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes: + *,$NAME_BAD) + die $(gettext 'git bisect $NAME_BAD' can take only one argument.) ;; H, doesn't this break i18n the big way? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] am --abort: revert changes introduced by failed 3way merge
Paul Tan pyoka...@gmail.com writes: Even when a merge conflict occurs with am --3way, the index will be modified with the results of any successfully merged files. These changes to the index will not be reverted with a git read-tree --reset -u HEAD ORIG_HEAD, as git read-tree will not be aware of how the current index differs from HEAD or ORIG_HEAD. To fix this, we first reset any conflicting entries in the index. The resulting index will contain the results of successfully merged files introduced by the failed merge. We write this index to a tree, and then use git read-tree to fast-forward this index tree back to ORIG_HEAD, thus undoing all the changes from the failed merge. When we are on an unborn branch, HEAD and ORIG_HEAD will not point to valid trees. In this case, use an empty tree. Signed-off-by: Paul Tan pyoka...@gmail.com --- git-am.sh | 6 +- t/t4151-am-abort.sh | 23 +++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index 67f4f25..e4fe3ed 100755 --- a/git-am.sh +++ b/git-am.sh @@ -519,7 +519,11 @@ then git rerere clear if safe_to_abort then - git read-tree --reset -u HEAD ORIG_HEAD + head_tree=$(git rev-parse --verify -q HEAD || echo $empty_tree) + git read-tree --reset -u $head_tree $head_tree + index_tree=$(git write-tree) + orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) + git read-tree -m -u $index_tree $orig_head git reset ORIG_HEAD What would the last reset do when ORIG_HEAD is invalid? In other words, does it make sense to worry about the case where ORIG_HEAD does not exist? At which point in the flow does it get written using what information? fi rm -fr $dotest diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index ea4a49e..2366042 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -70,6 +70,17 @@ test_expect_success 'am -3 --skip removes otherfile-4' ' test 4 = $(cat otherfile-4) git am --skip test_cmp_rev initial HEAD + test -z $(git ls-files -u) + test_path_is_missing otherfile-4 +' + +test_expect_success 'am -3 --abort removes otherfile-4' ' + git reset --hard initial + test_must_fail git am -3 0003-*.patch + test 3 -eq $(git ls-files -u | wc -l) + test 4 = $(cat otherfile-4) + git am --abort + test_cmp_rev initial HEAD test -z $(git ls-files -u) test_path_is_missing otherfile-4 ' @@ -102,4 +113,16 @@ test_expect_success 'am -3 --skip clears index on unborn branch' ' test_path_is_missing tmpfile ' +test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' ' + git checkout -f --orphan orphan + git reset + rm -f otherfile-4 file-1 + test_must_fail git am -3 0003-*.patch + test 2 -eq $(git ls-files -u | wc -l) + test 4 = $(cat otherfile-4) + git am --abort + test -z $(git ls-files -u) + test_path_is_missing otherfile-4 +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'
On 06/08/2015 10:51 PM, Matthieu Moy wrote: Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: On 06/08/2015 08:23 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: +/* Free all memory allocated for ref_array */ +void ref_array_clear(struct ref_array *array) Is this a private function? If so, then add static. If not, you probably want to export it in a .h file. It is in ref-filter.h. Ah, OK. It comes later in the series. Confused I am; if it comes later not in the same patch then it is not OK, is it? We could introduce ref-filter.h earlier, indeed. To me, the current solution is good enough, but introducing ref-filter.h early and adding function definition there in the same commit as you drop the static keyword for them would clearly be an improvement. But that would break the flow, wouldn't it? I wanted ref-filter to be introduced together, hence right after ref-filter.h we move code to ref-filter.c -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Michael Rappazzo rappa...@gmail.com writes:
A config option 'rebase.instructionFormat' can override the
default 'oneline' format of the rebase instruction list.
Since the list is parsed using the left, right or boundary mark plus
the sha1, they are prepended to the instruction format.
Signed-off-by: Michael Rappazzo rappa...@gmail.com
---
git-rebase--interactive.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..b92375e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,9 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H
to parse
+git rev-list $merges_option --format=%m%H ${format-%s} --reverse
--left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
while read -r sha1 rest
Looks OK, but
- Needs a patch to Documentation/config.txt and possibly also
Documentation/git-rebase.txt
- Needs tests somewhere in t34?? series.
Also I think git rev-list line has got way too long. As it is
already using backslash-continuation, do not hesitate to fold it
further, e.g.
git rev-list $merges_option --format=%m%H ${format-%s} \
--reverse --left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
while ...
Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1]: git-completion.tcsh fails w/ noclobber
tcsh users who happen to have 'set noclobber' elsewhere in their ~/.tcshrc or
~/.cshrc startup files get a 'File exist' error, and the tcsh completion file
doesn't get generated/updated. Adding a `!` in the redirect works correctly
for both clobber (default) and 'set noclobber' users.
Helped-by: Junio C Hamano notificati...@github.com
Mentored-by: Christian Couder christian.cou...@gmail.com
Signed-off-by: Ariel Faigon github.2...@yendor.com
---
contrib/completion/git-completion.tcsh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.tcsh
b/contrib/completion/git-completion.tcsh
index 6104a42..4a790d8 100644
--- a/contrib/completion/git-completion.tcsh
+++ b/contrib/completion/git-completion.tcsh
@@ -41,7 +41,7 @@ if ( ! -e ${__git_tcsh_completion_original_script} ) then
exit
endif
-cat EOF ${__git_tcsh_completion_script}
+cat EOF ! ${__git_tcsh_completion_script}
#!bash
#
# This script is GENERATED and will be overwritten automatically.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] bisect: simplify the add of new bisect terms
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes:
We create a file BISECT_TERMS in the repository .git to be read during a
bisection. The fonctions to be changed if we add new terms are quite
few.
In git-bisect.sh :
check_and_set_terms
bisect_voc
In bisect.c :
handle_bad_merge_base
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr
Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr
Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr
Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr
Signed-off-by: Huynh Khoi Nguyen Nguyen
huynh-khoi-nguyen.ngu...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
This step seems very straight-forward and makes sense from a cursory
look.
/*
+ * The terms used for this bisect session are stocked in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and stock them to adapt the messages
+ * accordingly. Default is bad/good.
+ */
s/stock/store/ perhaps? I think the idea is not to have this file
in the default case so that absence of it would mean you would be
looking for a transition from (older) good to (more recent) bad.
+void read_bisect_terms(void)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path(BISECT_TERMS);
+ FILE *fp = fopen(filename, r);
+
+ if (!fp) {
We might want to see why fopen() failed here. If it is because the
file did not exist, great. But otherwise?
diff --git a/git-bisect.sh b/git-bisect.sh
index 1f16aaf..529bb43 100644
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -77,6 +77,7 @@ bisect_start() {
orig_args=$(git rev-parse --sq-quote $@)
bad_seen=0
eval=''
+ start_bad_good=0
if test z$(git rev-parse --is-bare-repository) != zfalse
then
mode=--no-checkout
@@ -101,6 +102,9 @@ bisect_start() {
die $(eval_gettext '\$arg' does not appear to
be a valid revision)
break
}
+
+ start_bad_good=1
+
It is unclear what this variable means, or what it means to have
zero or one as its value.
case $bad_seen in
0) state='bad' ; bad_seen=1 ;;
*) state='good' ;;
@@ -172,6 +176,11 @@ bisect_start() {
}
git rev-parse --sq-quote $@ $GIT_DIR/BISECT_NAMES
eval $eval true
+ if test $start_bad_good -eq 1 -a ! -s $GIT_DIR/BISECT_TERMS
Avoid test condition1 -a condition2 (or -o).
+get_terms () {
+ if test -s $GIT_DIR/BISECT_TERMS
+ then
+ NAME_BAD=$(sed -n 1p $GIT_DIR/BISECT_TERMS)
+ NAME_GOOD=$(sed -n 2p $GIT_DIR/BISECT_TERMS)
It is sad that we need to open the file twice. Can't we do
something using read perhaps?
Don't we want to make sure these two names are sensible? We do not
want an empty-string, for example. I suspect you do not want to
take anything that check-ref-format does not like.
Same comment applies to the C code.
+bisect_voc () {
+ case $1 in
+ bad) echo bad ;;
+ good) echo good ;;
+ esac
+}
What is voc?
What if $1 is neither bad/good?
Did you mean to translate 'bad' to $NAME_BAD and 'good' to $NAME_GOOD?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 3/3] git-p4: fixing --changes-block-size handling
On Mon, Jun 8, 2015 at 11:43 AM, Luke Diamand l...@diamand.org wrote: On 08/06/15 18:18, Junio C Hamano wrote: Lex Spoon l...@lexspoon.org writes: Precisely, Junio, that's what I had in mind. The patch with the two lines deleted LGTM. Thanks, will do. I don't think we're quite there yet unfortunately. The current version of git-p4 will let you do things like: $ git p4 clone //depot@1,2015/05/31 i.e. get all the revisions between revision 1 and the end of last month. Because my change tries to batch up the revisions, it fails when presented with this. There aren't any test cases for this, but it's documented (briefly) in the manual page. I think that although the current code looks really nice and clean, it's going to have to pick up a bit more complexity to handle non-numerical revisions. I don't think it's possible to do batching at the same time. It shouldn't be too hard though; I'll look at it later this week. [jch: adding git@ back] Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] bisect: add the terms old/new
When not looking for a regression during a bisect but for a fix or a change in another given property, it can be confusing to use 'good' and 'bad'. This patch introduce `git bisect new` and `git bisect old` as an alternative to 'bad' and good': the commits which have a certain property must be marked as `new` and the ones which do not as `old`. The output will be the first commit after the change in the property. During a new/old bisect session you cannot use bad/good commands and vice-versa. `git bisect replay` works fine for old/new bisect sessions. Some commands are still not available for old/new: * git bisect start [new [old...]] is not possible: the commits will be treated as bad and good. * git rev-list --bisect does not treat the revs/bisect/new and revs/bisect/old-SHA1 files. * git bisect visualize seem to work partially: the tags are displayed correctly but the tree is not limited to the bisect section. Related discussions: - http://thread.gmane.org/gmane.comp.version-control.git/86063 introduced bisect fix unfixed to find fix. - http://thread.gmane.org/gmane.comp.version-control.git/182398 discussion around bisect yes/no or old/new. - http://thread.gmane.org/gmane.comp.version-control.git/199758 last discussion and reviews Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr Signed-off-by: Huynh Khoi Nguyen Nguyen huynh-khoi-nguyen.ngu...@ensimag.imag.fr Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr --- Documentation/git-bisect.txt | 48 ++-- bisect.c | 15 ++ git-bisect.sh| 32 +++-- t/t6030-bisect-porcelain.sh | 38 +++ 4 files changed, 116 insertions(+), 17 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4cb52a7..441a4bd 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -18,8 +18,8 @@ on the subcommand: git bisect help git bisect start [--no-checkout] [bad [good...]] [--] [paths...] - git bisect bad [rev] - git bisect good [rev...] + git bisect (bad|new) [rev] + git bisect (good|old) [rev...] git bisect skip [(rev|range)...] git bisect reset [commit] git bisect visualize @@ -104,6 +104,35 @@ For example, `git bisect reset HEAD` will leave you on the current bisection commit and avoid switching commits at all, while `git bisect reset bisect/bad` will check out the first bad revision. + +Alternative terms: bisect new and bisect old + + +If you are not at ease with the terms bad and good, perhaps +because you are looking for the commit that introduced a fix, you can +alternatively use new and old instead. +But note that you cannot mix bad and good with new and old. + + +git bisect new [rev] + + +Marks the commit as new, e.g. the bug is no longer there, if you are looking +for a commit that fixed a bug, or the feature that used to work is now broken +at this point, if you are looking for a commit that introduced a bug. +It is the equivalent of git bisect bad [rev]. + + +git bisect old [rev...] + + +Marks the commit as old, as the opposite of 'git bisect new'. +It is the equivalent of git bisect good [rev...]. + +You must run `git bisect start` without commits as argument and run +`git bisect new rev`/`git bisect old rev...` after to add the +commits. + Bisect visualize @@ -379,6 +408,21 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit has at least one parent whose reachable graph is fully traversable in the sense required by 'git pack objects'. +* Look for a fix instead of a regression in the code ++ + +$ git bisect start +$ git bisect new HEAD# current commit is marked as new +$ git bisect old HEAD~10 # the tenth commit from now is marked as old + ++ +If the last commit has a given property, and we are looking for the commit +which introduced this property. For each commit the bisection guide us to we +will test if the property is present, if it is we will mark the commit as new +with 'git bisect new', otherwise we will mark it as old. +At the end of the bisect session, the result will be the first new commit (e.g +the first one with the property). + SEE ALSO diff --git a/bisect.c b/bisect.c
Re: [PATCH] utf8.c: print warning about disabled iconv
Max Kirillov m...@max630.net writes: User, in theory, can be not the same person who builds, or can be not aware that the case needs recoding. Because you can pretty much say the same for build with iconv enabled, I think that line of argument is futile. The users do not have control over platform's iconv (and sysadmin's choice of locale packages) what charset/encoding can be converted to what other ones. I actually am OK if the user gets exactly the same warning between the two cases: - iconv failed to convert in the real reencode_string_len() - we compiled out iconv() and real conversion was asked. Does 'exactly the same' mean the same text? No, I was trying to point out the total lack of corresponding warnings in the iconv-enabled build. After all, if you had to convert between UTF-8 and ISO-2022-JP, the latter of which your system does not support, whether you use iconv-disabled build of Git or iconv-enabled build of Git, we pass the bytestream through, right? Your patch gives warning for the former (which is a good starting point if we want to warn user expected them to be converted, we didn't case) but does not do anything to the latter, even though users of the iconv-disabled build is more likely to be aware of the potential issue (and are likely to be willing to accept that) than the ones with iconv-enabled build that runs on a sysmet that cannot convert the specific encoding. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] status: factor two rebase-related messages together
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes:
---
wt-status.c | 30 +++---
1 file changed, 11 insertions(+), 19 deletions(-)
Hmmm, it obviously does not break anything but it is not obvious why
this is a good change.
Is it that you wanted to have a single instance of if on a branch,
we say 'you are rebasing that branch', otherwise we say 'you are
rebasing'? Even then, I am not sure if this code movement was the
best way to do so (an obvious alternative is to use a shared helper
function and call from the two arms of if/elseif/... chain).
diff --git a/wt-status.c b/wt-status.c
index 33452f1..fec6e85 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1032,7 +1032,7 @@ static void show_rebase_in_progress(struct wt_status *s,
{
struct stat st;
- if (has_unmerged(s)) {
+ if (has_unmerged(s) || state-rebase_in_progress ||
!stat(git_path(MERGE_MSG), st)) {
if (state-branch)
status_printf_ln(s, color,
_(You are currently rebasing branch
'%s' on '%s'.),
@@ -1042,25 +1042,17 @@ static void show_rebase_in_progress(struct wt_status
*s,
status_printf_ln(s, color,
_(You are currently rebasing.));
if (s-hints) {
- status_printf_ln(s, color,
- _( (fix conflicts and then run \git rebase
--continue\)));
- status_printf_ln(s, color,
- _( (use \git rebase --skip\ to skip this
patch)));
- status_printf_ln(s, color,
- _( (use \git rebase --abort\ to check out
the original branch)));
+ if (has_unmerged(s)) {
+ status_printf_ln(s, color,
+ _( (fix conflicts and then run \git
rebase --continue\)));
+ status_printf_ln(s, color,
+ _( (use \git rebase --skip\ to skip
this patch)));
+ status_printf_ln(s, color,
+ _( (use \git rebase --abort\ to
check out the original branch)));
+ } else
+ status_printf_ln(s, color,
+ _( (all conflicts fixed: run \git
rebase --continue\)));
}
- } else if (state-rebase_in_progress || !stat(git_path(MERGE_MSG),
st)) {
- if (state-branch)
- status_printf_ln(s, color,
- _(You are currently rebasing branch
'%s' on '%s'.),
- state-branch,
- state-onto);
- else
- status_printf_ln(s, color,
- _(You are currently rebasing.));
- if (s-hints)
- status_printf_ln(s, color,
- _( (all conflicts fixed: run \git rebase
--continue\)));
} else if (split_commit_in_progress(s)) {
if (state-branch)
status_printf_ln(s, color,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] git-checkout.txt: Document git checkout pathspec better
git checkout pathspec can be used to revert changes in the working tree. Signed-off-by: Torsten Bögershausen tbo...@web.de --- My first attempt to improve the documentation Documentation/git-checkout.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index d263a56..8cd018a 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -3,7 +3,7 @@ git-checkout(1) NAME -git-checkout - Checkout a branch or paths to the working tree +git-checkout - Switch branches or reverts changes in the working tree SYNOPSIS @@ -83,7 +83,8 @@ Omitting branch detaches HEAD at the tip of the current branch. When paths or `--patch` are given, 'git checkout' does *not* switch branches. It updates the named paths in the working tree from the index file or from a named tree-ish (most often a - commit). In this case, the `-b` and `--track` options are + commit). Changes in files are discarded and deleted files are + restored. In this case, the `-b` and `--track` options are meaningless and giving either of them results in an error. The tree-ish argument can be used to specify a specific tree-ish (i.e. commit, tag or tree) to update the index for the given -- 2.2.0.rc1.790.ge19fcd2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC v2 5/5] send-email: refactor address list process
Junio C Hamano gits...@pobox.com writes Matthieu Moy matthieu@grenoble-inp.fr writes: Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: Simplify code by creating a function to transform list of email lists (comma separated, with aliases ...) into a simple list of valid email addresses. I would have found the series easier to read if this refactoring came earlier (and then PATCH 2/5 would fix the bug as a positive side effect of the refactoring). I agree that doing 5/5 sooner would make 4/5 a lot clearer. Introducing the helper of 5/5 before 2/5 happens, and then replacing two calls to validate-address-list with process-address-list would hide the nature of the change, i.e. fixing a bug, so it is better to see it done before the refactoring of 5/5, provided if it is indeed a bug that these were not expanded. Ok thanks, I submit it again soon. Also I think we should swap the lines 'sanitize_address_list(...)' and 'expand_aliases(...)', i.e. first sanitize addresses and then expand aliases. We could then remove leading and trailing whitespaces in the sanitize_address_list function as aliases file formats supported by git send-email doesn't take these whitespace into account anyway: Example which currently can't work: git send-email --to= alias ... Moreover I think it's more natural to do that so. I'll do it right after the refactoring patch introducing process_address_list or maybe I should avoid changing this patch now ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] status: give more information during rebase -i
Guillaume Pagès guillaume.pa...@ensimag.grenoble-inp.fr writes:
git status gives more information during rebase -i, about the list of
command that are done during the rebase. It displays the two last
commands executed and the two next lines to be executed. It also gives
hints to find the whole files in .git directory.
---
Without 1/4 and 2/4 in the same thread, it is hard to guess what you
wanted to do with these two patches. Remember, reviewers review not
just your patches but those from many others.
I would in the meantime assume these are replacement patches for the
ones in
http://thread.gmane.org/gmane.comp.version-control.git/270743/focus=270744
diff --git a/wt-status.c b/wt-status.c
index c83eca5..7f88470 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1026,12 +1026,73 @@ static int split_commit_in_progress(struct wt_status
*s)
return split_in_progress;
}
+static void show_rebase_information(struct wt_status *s,
+ struct wt_status_state *state,
+ const char *color)
+{
+ if (state-rebase_interactive_in_progress) {
+ int i, begin;
+ int lines_to_show_nr = 2;
lines_to_show = 2 or nr_lines_to_show = 2 would be easier to read.
+
+ struct strbuf buf = STRBUF_INIT;
+ struct string_list have_done = STRING_LIST_INIT_DUP;
+ struct string_list yet_to_do = STRING_LIST_INIT_DUP;
+
+ strbuf_read_file(buf, git_path(rebase-merge/done), 0);
+ stripspace(buf, 1);
+ have_done.nr = string_list_split(have_done, buf.buf, '\n', -1);
+ string_list_remove_empty_items(have_done, 1);
+ strbuf_release(buf);
+
+ strbuf_read_file(buf,
git_path(rebase-merge/git-rebase-todo), 0);
+ stripspace(buf, 1);
+ string_list_split(yet_to_do, buf.buf, '\n', -1);
+ string_list_remove_empty_items(yet_to_do, 1);
+ strbuf_release(buf);
+
+ if (have_done.nr == 0)
+ status_printf_ln(s, color, _(No commands done.));
Do the users even need to be told that, I wonder?
+ else{
Style: else {
+ status_printf_ln(s, color,
+ Q_(Last command done (%d command done):,
+ Last commands done (%d commands
done):,
+ have_done.nr),
+ have_done.nr);
+ begin = (have_done.nr lines_to_show_nr) ?
have_done.nr-lines_to_show_nr : 0;
+ for (i = begin; i have_done.nr; i++) {
+ status_printf_ln(s, color,%s,
have_done.items[i].string);
+ }
Hmm, perhaps fold lines like this (and you do not need begin)?
for (i = (lines_to_show_nr have_done.nr)
? have_done.nr - lines_to_show_nr : 0;
i have_done.nr;
i++)
status_printf_ln(...);
+ if (have_done.nr lines_to_show_nr s-hints)
+status_printf_ln(s, color,
+ _( (see more in file %s)),
git_path(rebase-merge/done));
That's a nice touch ;-)
+ }
+ if (yet_to_do.nr == 0)
+ status_printf_ln(s, color,
+ _(No commands remaining.));
This I can see why we may want to say it.
+ else{
+
+ status_printf_ln(s, color,
+ Q_(Next command to do (%d remaining command):,
+ Next commands to do (%d remaining
commands):,
+ yet_to_do.nr),
+ yet_to_do.nr);
+ for (i = 0; i lines_to_show_nr i yet_to_do.nr;
i++) {
+ status_printf_ln(s, color,%s,
yet_to_do.items[i].string);
+ }
+ if (s-hints)
+status_printf_ln(s, color,
+ _( (use \git rebase --edit-todo\ to view
and edit)));
+ }
Make sure you do not leak memory used by two string lists here...
+ }
+}
+
static void show_rebase_in_progress(struct wt_status *s,
struct wt_status_state *state,
const char *color)
{
struct stat st;
+ show_rebase_information(s, state, color);
if (has_unmerged(s) || state-rebase_in_progress ||
!stat(git_path(MERGE_MSG), st)) {
if (state-branch)
status_printf_ln(s, color,
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info
Re: [PATCH 6/6] am --abort: keep unrelated commits on unborn branch
Paul Tan pyoka...@gmail.com writes:
Since 7b3b7e3 (am --abort: keep unrelated commits since the last failure
and warn, 2010-12-21), git-am would refuse to rewind HEAD if commits
were made since the last git-am failure. This check was implemented in
safe_to_abort(), which checked to see if HEAD's hash matched the
abort-safety file.
However, this check was skipped if the abort-safety file was empty,
which can happen if git-am failed while on an unborn branch.
Shouldn't we then be checking that the HEAD is still unborn if this
file is found and says that there was no history in the beginning,
in order to give the am on top of unborn workflow the same degree
of safety? Or is the fact that the file is empty sufficient not to
match any valid commit name that is 40-hex?
As such, if
any commits were made since then, they would be discarded. Fix this by
carrying on the abort safety check even if the abort-safety file is
empty.
Signed-off-by: Paul Tan pyoka...@gmail.com
---
git-am.sh | 2 +-
t/t4151-am-abort.sh | 11 +++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index 95f90a0..4324bb1 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -87,7 +87,7 @@ safe_to_abort () {
return 1
fi
- if ! test -s $dotest/abort-safety
+ if ! test -f $dotest/abort-safety
then
return 0
fi
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index b2a7fc5..833e7b2 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -142,4 +142,15 @@ test_expect_success 'am -3 --abort on unborn branch
removes applied commits' '
test refs/heads/orphan = $(git symbolic-ref HEAD)
'
+test_expect_success 'am --abort on unborn branch will keep local commits
intact' '
+ git checkout -f --orphan orphan
+ git reset
+ test_must_fail git am 0004-*.patch
+ test_commit unrelated2
+ git rev-parse HEAD expect
+ git am --abort
+ git rev-parse HEAD actual
+ test_cmp expect actual
+'
+
test_done
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] utf8.c: print warning about disabled iconv
Hi. On Mon, Jun 08, 2015 at 09:16:16AM -0700, Junio C Hamano wrote: Max Kirillov m...@max630.net writes: This gives undesirable result that returned data or even data written into repository is incorrect and user is not aware about it. I do not necessarily agree with that. The user knows what s/he is doing, data written to or shown from the repository is correct as far as the user is concerned, and the user takes the full respoinsibility when compiling out certain features. User, in theory, can be not the same person who builds, or can be not aware that the case needs recoding. It actually started when I compiled git without iconv support and got about 10 failed tests, and only 2 of them mentioned i18n in their name. Compiling out other features is not exactly the same. If user compiles out curl, for example, git will not be able to push or fetch through http, but it is not going to pretend to be working, it will fail visibly. I actually am OK if the user gets exactly the same warning between the two cases: - iconv failed to convert in the real reencode_string_len() - we compiled out iconv() and real conversion was asked. Does 'exactly the same' mean the same text? Shouldn't it describe the reason? I can see 2 possible failures in case of real iconv: unknown or unsupported encoding and invalid input. Wouldn't them better to be detailed in warning? -- Max -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/6] am --abort: support aborting to unborn branch
Paul Tan pyoka...@gmail.com writes: When git-am is first run on an unborn branch, no ORIG_HEAD is created. As such, any applied commits will remain even after a git am --abort. I think this answered my question on 4/6; that may indicate that these two are either done in a wrong order or perhaps should be a single change. To be consistent with the behavior of git am --abort when it is not run from an unborn branch, we empty the index, and then destroy the branch pointed to by HEAD if there is no ORIG_HEAD. Signed-off-by: Paul Tan pyoka...@gmail.com --- git-am.sh | 9 - t/t4151-am-abort.sh | 17 + 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/git-am.sh b/git-am.sh index e4fe3ed..95f90a0 100755 --- a/git-am.sh +++ b/git-am.sh @@ -524,7 +524,14 @@ then index_tree=$(git write-tree) orig_head=$(git rev-parse --verify -q ORIG_HEAD || echo $empty_tree) git read-tree -m -u $index_tree $orig_head - git reset ORIG_HEAD + if git rev-parse --verify -q ORIG_HEAD /dev/null 21 + then + git reset ORIG_HEAD + else + git read-tree $empty_tree + curr_branch=$(git symbolic-ref HEAD 2/dev/null) + git update-ref -d $curr_branch + fi fi rm -fr $dotest exit ;; diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh index 2366042..b2a7fc5 100755 --- a/t/t4151-am-abort.sh +++ b/t/t4151-am-abort.sh @@ -14,6 +14,7 @@ test_expect_success setup ' git add file-1 file-2 git commit -m initial git tag initial + git format-patch --stdout --root initial initial.patch for i in 2 3 4 5 6 do echo $i file-1 @@ -125,4 +126,20 @@ test_expect_success 'am -3 --abort removes otherfile-4 on unborn branch' ' test_path_is_missing otherfile-4 ' +test_expect_success 'am -3 --abort on unborn branch removes applied commits' ' + git checkout -f --orphan orphan + git reset + rm -f otherfile-4 otherfile-2 file-1 file-2 + test_must_fail git am -3 initial.patch 0003-*.patch + test 3 -eq $(git ls-files -u | wc -l) + test 4 = $(cat otherfile-4) + git am --abort + test -z $(git ls-files -u) + test_path_is_missing otherfile-4 + test_path_is_missing file-1 + test_path_is_missing file-2 + test 0 -eq $(git log --oneline 2/dev/null | wc -l) + test refs/heads/orphan = $(git symbolic-ref HEAD) +' + test_done -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] bisect: replace hardcoded bad|good by variables
To add new tags like old/new and have keywords less confusing, the
first step is to avoid hardcoding the keywords.
The default mode is still bad/good.
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr
Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr
Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr
Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr
Signed-off-by: Huynh Khoi Nguyen Nguyen
huynh-khoi-nguyen.ngu...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
git-bisect.sh | 47 ++-
1 file changed, 26 insertions(+), 21 deletions(-)
mode change 100755 = 100644 git-bisect.sh
diff --git a/git-bisect.sh b/git-bisect.sh
old mode 100755
new mode 100644
index ae3fec2..1f16aaf
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -32,6 +32,8 @@ OPTIONS_SPEC=
_x40='[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]'
_x40=$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40
+NAME_BAD=bad
+NAME_GOOD=good
bisect_head()
{
@@ -184,9 +186,12 @@ bisect_write() {
rev=$2
nolog=$3
case $state in
- bad)tag=$state ;;
- good|skip) tag=$state-$rev ;;
- *) die $(eval_gettext Bad bisect_write argument:
\$state) ;;
+ $NAME_BAD)
+ tag=$state ;;
+ $NAME_GOOD|skip)
+ tag=$state-$rev ;;
+ *)
+ die $(eval_gettext Bad bisect_write argument:
\$state) ;;
esac
git update-ref refs/bisect/$tag $rev || exit
echo # $state: $(git show-branch $rev) $GIT_DIR/BISECT_LOG
@@ -230,12 +235,12 @@ bisect_state() {
case $#,$state in
0,*)
die $(gettext Please call 'bisect_state' with at least one
argument.) ;;
- 1,bad|1,good|1,skip)
+ 1,$NAME_BAD|1,$NAME_GOOD|1,skip)
rev=$(git rev-parse --verify $(bisect_head)) ||
die $(gettext Bad rev input: $(bisect_head))
bisect_write $state $rev
check_expected_revs $rev ;;
- 2,bad|*,good|*,skip)
+ 2,$NAME_BAD|*,$NAME_GOOD|*,skip)
shift
hash_list=''
for rev in $@
@@ -249,8 +254,8 @@ bisect_state() {
bisect_write $state $rev
done
check_expected_revs $hash_list ;;
- *,bad)
- die $(gettext 'git bisect bad' can take only one argument.)
;;
+ *,$NAME_BAD)
+ die $(gettext 'git bisect $NAME_BAD' can take only one
argument.) ;;
*)
usage ;;
esac
@@ -259,21 +264,21 @@ bisect_state() {
bisect_next_check() {
missing_good= missing_bad=
- git show-ref -q --verify refs/bisect/bad || missing_bad=t
- test -n $(git for-each-ref refs/bisect/good-*) || missing_good=t
+ git show-ref -q --verify refs/bisect/$NAME_BAD || missing_bad=t
+ test -n $(git for-each-ref refs/bisect/$NAME_GOOD-*) ||
missing_good=t
case $missing_good,$missing_bad,$1 in
,,*)
- : have both good and bad - ok
+ : have both $NAME_GOOD and $NAME_BAD - ok
;;
*,)
# do not have both but not asked to fail - just report.
false
;;
- t,,good)
+ t,,$NAME_GOOD)
# have bad but not good. we could bisect although
# this is less optimum.
- gettextln Warning: bisecting only with a bad commit. 2
+ gettextln Warning: bisecting only with a $NAME_BAD commit. 2
if test -t 0
then
# TRANSLATORS: Make sure to include [Y] and [n] in your
@@ -283,7 +288,7 @@ bisect_next_check() {
read yesno
case $yesno in [Nn]*) exit 1 ;; esac
fi
- : bisect without good...
+ : bisect without $NAME_GOOD...
;;
*)
@@ -307,7 +312,7 @@ bisect_auto_next() {
bisect_next() {
case $# in 0) ;; *) usage ;; esac
bisect_autostart
- bisect_next_check good
+ bisect_next_check $NAME_GOOD
# Perform all bisection computation, display and checkout
git bisect--helper --next-all $(test -f $GIT_DIR/BISECT_HEAD echo
--no-checkout)
@@ -316,15 +321,15 @@ bisect_next() {
# Check if we should exit because bisection is finished
if test $res -eq 10
then
- bad_rev=$(git show-ref --hash --verify refs/bisect/bad)
+ bad_rev=$(git show-ref --hash --verify refs/bisect/$NAME_BAD)
bad_commit=$(git show-branch $bad_rev)
echo # first bad commit: $bad_commit $GIT_DIR/BISECT_LOG
[PATCH 1/4] bisect : correction of typo
--- bisect.c| 2 +- t/t6030-bisect-porcelain.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index 10f5e57..de92953 100644 --- a/bisect.c +++ b/bisect.c @@ -743,7 +743,7 @@ static void handle_bad_merge_base(void) fprintf(stderr, Some good revs are not ancestor of the bad rev.\n git bisect cannot work properly in this case.\n - Maybe you mistake good and bad revs?\n); + Maybe you mistook good and bad revs?\n); exit(1); } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 06b4868..9e2c203 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -362,7 +362,7 @@ test_expect_success 'bisect starting with a detached HEAD' ' test_expect_success 'bisect errors out if bad and good are mistaken' ' git bisect reset test_must_fail git bisect start $HASH2 $HASH4 2 rev_list_error - grep mistake good and bad rev_list_error + grep mistook good and bad rev_list_error git bisect reset ' -- 2.4.1.414.ge7a9de3.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] bisect: simplify the add of new bisect terms
We create a file BISECT_TERMS in the repository .git to be read during a
bisection. The fonctions to be changed if we add new terms are quite
few.
In git-bisect.sh :
check_and_set_terms
bisect_voc
In bisect.c :
handle_bad_merge_base
Signed-off-by: Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr
Signed-off-by: Louis Stuber stub...@ensimag.grenoble-inp.fr
Signed-off-by: Valentin Duperray valentin.duper...@ensimag.imag.fr
Signed-off-by: Franck Jonas franck.jo...@ensimag.imag.fr
Signed-off-by: Lucien Kong lucien.k...@ensimag.imag.fr
Signed-off-by: Thomas Nguy thomas.n...@ensimag.imag.fr
Signed-off-by: Huynh Khoi Nguyen Nguyen
huynh-khoi-nguyen.ngu...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
bisect.c | 65 ---
git-bisect.sh | 58
2 files changed, 103 insertions(+), 20 deletions(-)
diff --git a/bisect.c b/bisect.c
index de92953..3b7df85 100644
--- a/bisect.c
+++ b/bisect.c
@@ -21,6 +21,9 @@ static const char *argv_checkout[] = {checkout, -q, NULL,
--, NULL};
static const char *argv_show_branch[] = {show-branch, NULL, NULL};
static const char *argv_update_ref[] = {update-ref, --no-deref,
BISECT_HEAD, NULL, NULL};
+static const char *name_bad;
+static const char *name_good;
+
/* Remember to update object flag allocation in object.h */
#define COUNTED(1u16)
@@ -403,7 +406,7 @@ struct commit_list *find_bisection(struct commit_list *list,
static int register_ref(const char *refname, const unsigned char *sha1,
int flags, void *cb_data)
{
- if (!strcmp(refname, bad)) {
+ if (!strcmp(refname, name_bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
hashcpy(current_bad_oid-hash, sha1);
} else if (starts_with(refname, good-)) {
@@ -634,7 +637,7 @@ static void exit_if_skipped_commits(struct commit_list
*tried,
return;
printf(There are only 'skip'ped commits left to test.\n
- The first bad commit could be any of:\n);
+ The first %s commit could be any of:\n, name_bad);
print_commit_list(tried, %s\n, %s\n);
if (bad)
printf(%s\n, oid_to_hex(bad));
@@ -732,18 +735,19 @@ static void handle_bad_merge_base(void)
if (is_expected_rev(current_bad_oid)) {
char *bad_hex = oid_to_hex(current_bad_oid);
char *good_hex = join_sha1_array_hex(good_revs, ' ');
-
- fprintf(stderr, The merge base %s is bad.\n
- This means the bug has been fixed
- between %s and [%s].\n,
- bad_hex, bad_hex, good_hex);
-
+ if (!strcmp(name_bad, bad)) {
+ fprintf(stderr, The merge base %s is bad.\n
+ This means the bug has been fixed
+ between %s and [%s].\n,
+ bad_hex, bad_hex, good_hex);
+ }
exit(3);
}
- fprintf(stderr, Some good revs are not ancestor of the bad rev.\n
+ fprintf(stderr, Some %s revs are not ancestor of the %s rev.\n
git bisect cannot work properly in this case.\n
- Maybe you mistook good and bad revs?\n);
+ Maybe you mistook %s and %s revs?\n,
+ name_good, name_bad, name_good, name_bad);
exit(1);
}
@@ -755,10 +759,10 @@ static void handle_skipped_merge_base(const unsigned char
*mb)
warning(the merge base between %s and [%s]
must be skipped.\n
- So we cannot be sure the first bad commit is
+ So we cannot be sure the first %s commit is
between %s and %s.\n
We continue anyway.,
- bad_hex, good_hex, mb_hex, bad_hex);
+ bad_hex, good_hex, name_bad, mb_hex, bad_hex);
free(good_hex);
}
@@ -839,7 +843,7 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
int fd;
if (!current_bad_oid)
- die(a bad revision is needed);
+ die(a %s revision is needed, name_bad);
/* Check if file BISECT_ANCESTORS_OK exists. */
if (!stat(filename, st) S_ISREG(st.st_mode))
@@ -890,6 +894,31 @@ static void show_diff_tree(const char *prefix, struct
commit *commit)
}
/*
+ * The terms used for this bisect session are stocked in
+ * BISECT_TERMS: it can be bad/good or new/old.
+ * We read them and stock them to adapt the messages
+ * accordingly. Default is bad/good.
+ */
+void read_bisect_terms(void)
+{
+ struct strbuf str = STRBUF_INIT;
+ const char *filename = git_path(BISECT_TERMS);
+ FILE *fp = fopen(filename, r);
+
+ if (!fp) {
+ name_bad = bad;
+ name_good
Re: [PATCH 4/4] bisect: add the terms old/new
Antoine Delaite antoine.dela...@ensimag.grenoble-inp.fr writes:
When not looking for a regression during a bisect but for a fix or a
change in another given property, it can be confusing to use 'good'
and 'bad'.
This patch introduce `git bisect new` and `git bisect old` as an
alternative to 'bad' and good': the commits which have a certain
property must be marked as `new` and the ones which do not as `old`.
The output will be the first commit after the change in the property.
During a new/old bisect session you cannot use bad/good commands and
vice-versa.
`git bisect replay` works fine for old/new bisect sessions.
Some commands are still not available for old/new:
* git bisect start [new [old...]] is not possible: the
commits will be treated as bad and good.
Just throwing a suggestion. You could perhaps add a new verb to be
used before starting to do anything, e.g.
$ git bisect terms new old
(where the default mode is git bisect terms bad good) to set up
the terms, and then after that
$ git bisect start $new $old1 $old2...
would be treated as you would naturally expect, no?
* git rev-list --bisect does not treat the revs/bisect/new and
revs/bisect/old-SHA1 files.
That shouldn't be hard to add, I would imagine, either by
git rev-list --bisect=new,old
or teaching git rev-list --bisect to read the terms itself, as a
follow-up patch.
* git bisect visualize seem to work partially: the tags are
displayed correctly but the tree is not limited to the bisect
section.
This is directly related to the lack of git rev-list --bisect
support, I would imagine. If you make the command read terms, I
suspect that it would solve itself.
@@ -379,6 +408,21 @@ In this case, when 'git bisect run' finishes, bisect/bad
will refer to a commit
has at least one parent whose reachable graph is fully traversable in the
sense
required by 'git pack objects'.
+* Look for a fix instead of a regression in the code
++
+
+$ git bisect start
+$ git bisect new HEAD# current commit is marked as new
+$ git bisect old HEAD~10 # the tenth commit from now is marked as old
+
++
+If the last commit has a given property, and we are looking for the commit
+which introduced this property.
Hmph, that is not a complete sentence and I cannot understand what
it is trying to say.
+For each commit the bisection guide us to we
s/guide us to we/guides us to, we/;
+will test if the property is present, if it is we will mark the commit as new
+with 'git bisect new', otherwise we will mark it as old.
It would be easier to read as separate sentences, i.e.
s/is present, if it is/is present. If it is/;
diff --git a/bisect.c b/bisect.c
index 3b7df85..511b905 100644
--- a/bisect.c
+++ b/bisect.c
@@ -409,7 +409,8 @@ static int register_ref(const char *refname, const
unsigned char *sha1,
if (!strcmp(refname, name_bad)) {
current_bad_oid = xmalloc(sizeof(*current_bad_oid));
hashcpy(current_bad_oid-hash, sha1);
- } else if (starts_with(refname, good-)) {
+ } else if (starts_with(refname, good-) ||
+starts_with(refname, old-)) {
sha1_array_append(good_revs, sha1);
} else if (starts_with(refname, skip-)) {
sha1_array_append(skipped_revs, sha1);
@@ -741,6 +742,12 @@ static void handle_bad_merge_base(void)
between %s and [%s].\n,
bad_hex, bad_hex, good_hex);
}
+ else if (!strcmp(name_bad, new)) {
+ fprintf(stderr, The merge base %s is new.\n
+ The property has changed
+ between %s and [%s].\n,
+ bad_hex, bad_hex, good_hex);
+ }
After reading the previous patches in the series, I expected that
this new code would know to read terms and does not limit us only
to new and old. Somewhat disappointing.
@@ -439,7 +441,7 @@ bisect_replay () {
start)
cmd=bisect_start $rev
eval $cmd ;;
- good|bad|skip)
+ good|bad|skip|old|new)
Not NAME_GOOD / NAME_BAD?
To support replay, you may want to consider the bisect terms
approach and when the bisection is using non-standard terms record
that as the first entry to the bisect log.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Difference between v1 and v2 of this patch: - Fixed indentation from spaces to match the existing style - Changed the prepended sha1 from short (%h) to long (%H) - Used bash variable default when the config option is not present Michael Rappazzo (1): git-rebase--interactive.sh: add config option for custom instruction format git-rebase--interactive.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) -- 2.4.2 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
A config option 'rebase.instructionFormat' can override the
default 'oneline' format of the rebase instruction list.
Since the list is parsed using the left, right or boundary mark plus
the sha1, they are prepended to the instruction format.
Signed-off-by: Michael Rappazzo rappa...@gmail.com
---
git-rebase--interactive.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index dc3133f..b92375e 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -977,7 +977,9 @@ else
revisions=$onto...$orig_head
shortrevisions=$shorthead
fi
-git rev-list $merges_option --pretty=oneline --reverse --left-right
--topo-order \
+format=$(git config --get rebase.instructionFormat)
+# the 'rev-list .. | sed' requires %m to parse; the instruction requires %H to
parse
+git rev-list $merges_option --format=%m%H ${format-%s} --reverse
--left-right --topo-order \
$revisions ${restrict_revision+^$restrict_revision} | \
sed -n s/^//p |
while read -r sha1 rest
--
2.4.2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: GIT for Microsoft Access projects
-Original Message- From: git-ow...@vger.kernel.org [mailto:git-ow...@vger.kernel.org] On Behalf Of Konstantin Khomoutov Sent: June 8, 2015 12:15 PM To: hack...@suddenlink.net Cc: git@vger.kernel.org Subject: Re: GIT for Microsoft Access projects On Mon, 8 Jun 2015 9:45:17 -0500 hack...@suddenlink.net wrote: [...] My question is, will GIT work with MS access forms, queries, tables, modules, etc? [...] Git works with files. So in principle it will work with *files* containing your MS access stuff. But Git will consider and treat those files as opaque blobs of data. That is, you will get no fancy diffing like asking Git to graphically (or otherwise) show you what exact changes have been made to a particular form or query between versions X and Y of a given MS access document -- all it will be able to show you is commit messages describing those changes. So... If you're fine with this setting, Git will work for you, but if not, it won't. One last note: are you really sure you want an SCM/VCS tool to manage your files and not a document management system (DMS) instead? I mean stuff like Alfresco (free software by the way) and the like. Consider also what you are specifically managing in MS Access. Are you looking for management of configuration data, like settings, properties, and such, or is this transactional or user-related. If managing environment-specific content, it may be worth storing raw SQL INSERT statements, with appropriate variable references, or export to XML/CSV, and having those in git so that the purpose for configuration-like data can be explained and diff'ed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] git-rebase--interactive.sh: add config option for custom instruction format
Michael Rappazzo rappa...@gmail.com writes: Difference between v1 and v2 of this patch: - Fixed indentation from spaces to match the existing style - Changed the prepended sha1 from short (%h) to long (%H) - Used bash variable default when the config option is not present Michael Rappazzo (1): git-rebase--interactive.sh: add config option for custom instruction format git-rebase--interactive.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) Does this pass tests? I see many failures including t3415. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/13] delete_ref(): move declaration to refs.h
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
Also
* Add a docstring
* Rename the second parameter to old_sha1, to be consistent with the
convention used elsewhere in the refs module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
cache.h | 2 --
refs.c | 5 +++--
refs.h | 9 +
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 571c98f..be92121 100644
--- a/cache.h
+++ b/cache.h
@@ -585,8 +585,6 @@ extern void update_index_if_able(struct index_state *,
struct lock_file *);
extern int hold_locked_index(struct lock_file *, int);
extern void set_alternate_index_output(const char *);
-extern int delete_ref(const char *, const unsigned char *sha1, unsigned int
flags);
-
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
extern int trust_ctime;
diff --git a/refs.c b/refs.c
index a742d79..b575bb8 100644
--- a/refs.c
+++ b/refs.c
@@ -2789,7 +2789,8 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
return 0;
}
-int delete_ref(const char *refname, const unsigned char *sha1, unsigned int
flags)
+int delete_ref(const char *refname, const unsigned char *old_sha1,
+ unsigned int flags)
{
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
@@ -2797,7 +2798,7 @@ int delete_ref(const char *refname, const unsigned char
*sha1, unsigned int flag
transaction = ref_transaction_begin(err);
if (!transaction ||
ref_transaction_delete(transaction, refname,
- (sha1 !is_null_sha1(sha1)) ? sha1 :
NULL,
+ (old_sha1 !is_null_sha1(old_sha1)) ?
old_sha1 : NULL,
flags, NULL, err) ||
ref_transaction_commit(transaction, err)) {
error(%s, err.buf);
diff --git a/refs.h b/refs.h
index 8c3d433..8785bca 100644
--- a/refs.h
+++ b/refs.h
@@ -202,6 +202,15 @@ extern int read_ref_at(const char *refname, unsigned int
flags,
/** Check if a particular reflog exists */
extern int reflog_exists(const char *refname);
+/*
+ * Delete the specified reference. If old_sha1 is non-NULL and not
+ * NULL_SHA1, then verify that the current value of the reference is
+ * old_sha1 before deleting it.
And here I wondered what the distinction between NULL and non-NULL,
but NULL_SHA1
is and digging into the code, there is none. (As you can also see in
this patch above with
(old_sha1 !is_null_sha1(old_sha1)) ? old_sha1 : NULL,
but when digging deeper, the !is_null_sha1(old_sha1) is an arbitrary
limitation (i.e.
ref_transaction_delete and ref_transaction_update don't differ between
those two cases.)
The distinction comes in at lock_ref_sha1_basic, where I think we may
want to get rid of
the is_null_sha1 check and depend on NULL/non-NULL as the difference
for valid and invalid
sha1's?
That said, do we want to add another sentence to the doc here saying
non-NULL and not
NULL_SHA1 are treated the same or is it clear enough?
With or without this question addressed:
Reviewed-by: Stefan Beller sbel...@google.com
flags is passed to
+ * ref_transaction_delete().
+ */
+extern int delete_ref(const char *refname, const unsigned char *old_sha1,
+ unsigned int flags);
+
/** Delete a reflog */
extern int delete_reflog(const char *refname);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fsck: report errors if reflog entries point at invalid objects
On Mon, Jun 08, 2015 at 06:00:09PM +0200, Johannes Schindelin wrote: I like the idea, but I am a bit uncertain whether it would constitute too backwards-incompatible a change to make this an error. I think it could be argued both ways: it *is* an improvement, but it could also possibly disrupt scripts that work pretty nicely at the moment. What kind of script are you worried about? I was concerned about scripts that work on repositories whose reflogs become inconsistent for whatever reason (that happened a lot to me in the past, IIRC it had something to do with bare repositories and/or shared object databases). I think these repositories are already broken. You cannot run `git gc` in such a repository, as it will barf when trying to walk the reflog tips during `git repack`. We run into this exact situation at GitHub because of our shared object databases. Our per-fork repack code basically has to do: if ! git repack ...; then git reflog expire --expire-unreachable=all --all git repack ... || die ok, it really is broken fi -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] prune_remote(): use delete_refs()
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
This will result in errors being emitted for references that can't be
deleted, but that is a good thing.
This sounds a bit like hand-waving to me. Trust me, I'm an engineer!.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/remote.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index c8dc724..cc3c741 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1314,19 +1314,12 @@ static int prune_remote(const char *remote, int
dry_run)
string_list_append(refs_to_prune, item-util);
string_list_sort(refs_to_prune);
- if (!dry_run) {
- struct strbuf err = STRBUF_INIT;
- if (repack_without_refs(refs_to_prune, err))
- result |= error(%s, err.buf);
- strbuf_release(err);
- }
+ if (!dry_run)
+ result |= delete_refs(refs_to_prune);
for_each_string_list_item(item, states.stale) {
const char *refname = item-util;
- if (!dry_run)
- result |= delete_ref(refname, NULL, 0);
-
if (dry_run)
printf_ln(_( * [would prune] %s),
abbrev_ref(refname, refs/remotes/));
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/13] Improve refs module encapsulation
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote: Add functions to the reference API to * Delete a bunch of references at once, but *without* failing the whole transaction if one of the deletions fails. This functionality is used by `git remote remove` and `git remote prune`. * Create initial references during `git clone`. (During clone, references are written directly to the `packed-refs` file without any locking.) Also move the remaining refs function declarations from `cache.h` to `refs.h`. This improves the encapsulation of the refs module. Especially, it means that code outside of the refs module should no longer need to care about the distinction between loose and packed references. These patches are also available from my GitHub account [1] as branch init-delete-refs-api. [1] https://github.com/mhagger/git Thw whole series looks good to me. Michael Haggerty (13): delete_ref(): move declaration to refs.h remove_branches(): remove temporary delete_ref(): handle special case more explicitly delete_refs(): new function for the refs API delete_refs(): improve error message delete_refs(): convert error message to lower case prune_remote(): use delete_refs() repack_without_refs(): make function private initial_ref_transaction_commit(): function for initial ref creation refs: remove some functions from the module's public interface initial_ref_transaction_commit(): check for duplicate refs initial_ref_transaction_commit(): check for ref D/F conflicts refs: move the remaining ref module declarations to refs.h archive.c | 1 + builtin/blame.c | 1 + builtin/clone.c | 19 - builtin/fast-export.c | 1 + builtin/fmt-merge-msg.c | 1 + builtin/init-db.c | 1 + builtin/log.c | 1 + builtin/remote.c| 33 +--- cache.h | 68 refs.c | 167 +++--- refs.h | 210 +++- remote-testsvn.c| 1 + 12 files changed, 316 insertions(+), 188 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 09/11] ref-filter: move code from 'for-each-ref'
Karthik Nayak karthik@gmail.com writes: --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -1129,7 +56,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) memset(ref_cbdata, 0, sizeof(ref_cbdata)); ref_cbdata.filter.name_patterns = argv; - for_each_rawref(ref_filter_handler, ref_cbdata); + filter_refs(for_each_rawref, ref_cbdata); This seems unrelated from the rest of the patch. And you haven't introduced filter_refs yet! -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fsck: report errors if reflog entries point at invalid objects
Hi Peff, On 2015-06-08 18:56, Jeff King wrote: On Mon, Jun 08, 2015 at 06:00:09PM +0200, Johannes Schindelin wrote: I like the idea, but I am a bit uncertain whether it would constitute too backwards-incompatible a change to make this an error. I think it could be argued both ways: it *is* an improvement, but it could also possibly disrupt scripts that work pretty nicely at the moment. What kind of script are you worried about? I was concerned about scripts that work on repositories whose reflogs become inconsistent for whatever reason (that happened a lot to me in the past, IIRC it had something to do with bare repositories and/or shared object databases). I think these repositories are already broken. You cannot run `git gc` in such a repository, as it will barf when trying to walk the reflog tips during `git repack`. We run into this exact situation at GitHub because of our shared object databases. Our per-fork repack code basically has to do: if ! git repack ...; then git reflog expire --expire-unreachable=all --all git repack ... || die ok, it really is broken fi Good point. So if I needed any more convincing that Michael's patch is a bug fix (as opposed to a backwards-incompatible change), this did it. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5] am: teach mercurial patch parser how to read from stdin
On Mon, Jun 8, 2015 at 8:48 AM, Paul Tan pyoka...@gmail.com wrote:
git-mailsplit, which splits mbox patches, will read the patch from stdin
when the filename is - or there are no files listed on the
command-line.
To be consistent with this behavior, teach the mercurial patch parser to
read from stdin if the filename is - or no files are listed on the
command-line.
Based-on-patch-by: Chris Packham judge.pack...@gmail.com
Signed-off-by: Paul Tan pyoka...@gmail.com
The whole series looks good to me.
Thanks,
Stefan
---
git-am.sh | 4 +++-
t/t4150-am.sh | 10 ++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/git-am.sh b/git-am.sh
index d97da85..0a40d46 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -327,6 +327,7 @@ split_patches () {
;;
hg)
this=0
+ test 0 -eq $# set -- -
for hg in $@
do
this=$(( $this + 1 ))
@@ -338,6 +339,7 @@ split_patches () {
# Since we cannot guarantee that the commit message
is in
# git-friendly format, we put no Subject: line and
just consume
# all of the message as the body
+ cat $hg |
LANG=C LC_ALL=C @@PERL@@ -M'POSIX qw(strftime)' -ne
'BEGIN { $subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ;
print ; }
@@ -353,7 +355,7 @@ split_patches () {
print \n, $_ ;
$subject = 1;
}
- ' $hg $dotest/$msgnum || clean_abort
+ ' $dotest/$msgnum || clean_abort
done
echo $this $dotest/last
this=
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 4beb4b3..3ebafd9 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -259,6 +259,16 @@ test_expect_success 'am applies hg patch' '
test_cmp_rev second^ HEAD^
'
+test_expect_success 'am --patch-format=hg applies hg patch' '
+ rm -fr .git/rebase-apply
+ git checkout -f first
+ git am --patch-format=hg patch1-hg.eml
+ test_path_is_missing .git/rebase-apply
+ git diff --exit-code second
+ test_cmp_rev second HEAD
+ test_cmp_rev second^ HEAD^
+'
+
test_expect_success 'setup: new author and committer' '
GIT_AUTHOR_NAME=Another Thor
GIT_AUTHOR_EMAIL=a.t...@example.com
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 09/11] ref-filter: move code from 'for-each-ref'
On 06/08/2015 10:38 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -1129,7 +56,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) memset(ref_cbdata, 0, sizeof(ref_cbdata)); ref_cbdata.filter.name_patterns = argv; -for_each_rawref(ref_filter_handler, ref_cbdata); +filter_refs(for_each_rawref, ref_cbdata); This seems unrelated from the rest of the patch. And you haven't introduced filter_refs yet! That definitely is, this happened after fixing merges I suppose. Ignore it for now, I'll fix it. -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/13] prune_remote(): use delete_refs()
On Mon, Jun 08, 2015 at 09:57:04AM -0700, Stefan Beller wrote: On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This will result in errors being emitted for references that can't be deleted, but that is a good thing. This sounds a bit like hand-waving to me. Trust me, I'm an engineer!. I think the argument is we failed to do that the user asked for, but never reported the reason why. But I don't see how that is the case. We already complain if repack_without_refs fail, and AFAICT the original call to delete_ref() would emit an error, as well. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/11] create ref-filter from for-each-ref
On 06/08/2015 10:45 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: Version four of this patch can be found here : http://www.mail-archive.com/git@vger.kernel.org/msg70280.html v5 : http://www.mail-archive.com/git@vger.kernel.org/msg70888.html Changes in v5: *Rename functions to better suit the code. *implement filter_refs() *use FLEX_ARRAY for refname *other small changes Changes in v6: *based on latest master branch (github.com/git/git) *rename variables named sort to sorting. Other than the hunk in PATCH 9 that belongs to PATCH 10, and the misleading commit message in PATCH 2, the series looks good to me. (BTW, an easy way to check that each patch is compilable is to run git rebase -i --exec make. It would have noticed the issue with filter_refs() automatically) Thats brilliant, just what I needed! Thanks a lot :D -- Regards, Karthik -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] read-cache: fix untracked cache invalidation when split-index is used
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes: Before this change, t7063.17 fails. The actual action though happens at t7063.16 where the entry two is added back to index after being removed in the .13. Here we expect a directory invalidate at .16 and none at .17 where untracked cache is refreshed. But things do not go as expected when GIT_TEST_SPLIT_INDEX is set. The different behavior that happens at .16 when split index is used: the entry two, when deleted at .13, is simply marked deleted. When .16 executes, the entry resurfaces from the version in base index. This happens in merge_base_index() where add_index_entry() is called to add two back from the base index. This is where the bug comes from. The add_index_entry() is called with ADD_CACHE_KEEP_CACHE_TREE flag because this version of two is not new, it does not break either cache-tree or untracked cache. The code should check this flag and not invalidate untracked cache. This causes a second invalidation violates test expectation. The fix is obvious. Noticed-by: Thomas Gummerer t.gumme...@gmail.com Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com --- PS. perhaps I should rename ADD_CACHE_KEEP_CACHE_TREE to ADD_CACHE_KEEP_CACHE, or add a new flag .._KEEP_UNTRACKED_CACHE to avoid confusion. That may not be a bad idea, indeed. Untracked cache, split index and the last part (*) all aim at a smaller user audience with large work trees. They are not really what a common user needs, but I hope they do have users. I do hope that this can be made for everybody, though. Any project will get larger, not smaller, over time and these two (I am not sure what you refer to as 'last part', though) are meant to support projects with larger working trees better. After all, that is why I merged the untracked-cache series reasonably early to 'master' in this cycle to give us time to shake out little issues like this one. I think we killed two so far since it has been merged to 'master', so the plan is working rather nicely ;-). Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/13] delete_refs(): convert error message to lower case
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This string is going to have to be re-internationalized anyway because of the previous commit. So while we're at it, we might as well convert it to lower case as per our usual practice. Although the previous patch and this are addressing two slightly different things, we may want to squash this into the previous without dropping any of the commit message? (It might make reviewing easier, I'd assume) Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index 2a2a06d..a10aba8 100644 --- a/refs.c +++ b/refs.c @@ -2827,7 +2827,7 @@ int delete_refs(struct string_list *refnames) const char *refname = refnames-items[i].string; if (delete_ref(refname, NULL, 0)) - result |= error(_(Could not remove reference %s), refname); + result |= error(_(could not remove reference %s), refname); } return result; -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 03/13] delete_ref(): handle special case more explicitly
On Mon, Jun 8, 2015 at 4:45 AM, Michael Haggerty mhag...@alum.mit.edu wrote:
delete_ref() uses a different convention for its old_sha1 parameter
than, say, ref_transaction_delete(): NULL_SHA1 means not to check the
old value. Make this fact a little bit clearer in the code by handling
it in explicit, commented code rather than burying it in a conditional
expression.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index b575bb8..f9d87b6 100644
--- a/refs.c
+++ b/refs.c
@@ -2795,10 +2795,13 @@ int delete_ref(const char *refname, const unsigned
char *old_sha1,
struct ref_transaction *transaction;
struct strbuf err = STRBUF_INIT;
+ /* Treat NULL_SHA1 as don't care */
and by don't care you mean we still care about getting it deleted,
the part we don't care about is the particular sha1 (could be a bogus ref).
+ if (old_sha1 is_null_sha1(old_sha1))
+ old_sha1 = NULL;
+
transaction = ref_transaction_begin(err);
if (!transaction ||
- ref_transaction_delete(transaction, refname,
- (old_sha1 !is_null_sha1(old_sha1)) ?
old_sha1 : NULL,
+ ref_transaction_delete(transaction, refname, old_sha1,
flags, NULL, err) ||
ref_transaction_commit(transaction, err)) {
error(%s, err.buf);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/6] am --skip/--abort: improve index/worktree cleanup
On Sat, Jun 6, 2015 at 4:46 AM, Paul Tan pyoka...@gmail.com wrote: Currently git-am attempts to clean up the index/worktree when skipping or aborting, but does not do it very well: * While it discards conflicted index entries, it does not remove any other modifications made to the index due to a previous threeway merge. * It expects HEAD/ORIG_HEAD to exist, and thus does not clean up the index when on an unborn branch. This patch series addresses the above two general problems by making the calls to git-read-tree aware of the differences between our current index and HEAD/ORIG_HEAD, and by explictly defining what happens when we are on an unborn branch. Paul Tan (6): am --skip: revert changes introduced by failed 3way merge am -3: support 3way merge on unborn branch am --skip: support skipping while on unborn branch am --abort: revert changes introduced by failed 3way merge am --abort: support aborting to unborn branch am --abort: keep unrelated commits on unborn branch The whole series looks good to me. Thanks, Stefan git-am.sh | 31 ++-- t/t4151-am-abort.sh | 81 + 2 files changed, 104 insertions(+), 8 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'
Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: On 06/08/2015 08:23 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: +/* Free all memory allocated for ref_array */ +void ref_array_clear(struct ref_array *array) Is this a private function? If so, then add static. If not, you probably want to export it in a .h file. It is in ref-filter.h. Ah, OK. It comes later in the series. Confused I am; if it comes later not in the same patch then it is not OK, is it? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/11] create ref-filter from for-each-ref
Karthik Nayak karthik@gmail.com writes: Version four of this patch can be found here : http://www.mail-archive.com/git@vger.kernel.org/msg70280.html v5 : http://www.mail-archive.com/git@vger.kernel.org/msg70888.html Changes in v5: *Rename functions to better suit the code. *implement filter_refs() *use FLEX_ARRAY for refname *other small changes Changes in v6: * based on latest master branch (github.com/git/git) * rename variables named sort to sorting. Other than the hunk in PATCH 9 that belongs to PATCH 10, and the misleading commit message in PATCH 2, the series looks good to me. (BTW, an easy way to check that each patch is compilable is to run git rebase -i --exec make. It would have noticed the issue with filter_refs() automatically) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v5 05/10] for-each-ref: introduce 'ref_array_clear()'
Junio C Hamano gits...@pobox.com writes: Matthieu Moy matthieu@grenoble-inp.fr writes: Karthik Nayak karthik@gmail.com writes: On 06/08/2015 08:23 PM, Matthieu Moy wrote: Karthik Nayak karthik@gmail.com writes: +/* Free all memory allocated for ref_array */ +void ref_array_clear(struct ref_array *array) Is this a private function? If so, then add static. If not, you probably want to export it in a .h file. It is in ref-filter.h. Ah, OK. It comes later in the series. Confused I am; if it comes later not in the same patch then it is not OK, is it? We could introduce ref-filter.h earlier, indeed. To me, the current solution is good enough, but introducing ref-filter.h early and adding function definition there in the same commit as you drop the static keyword for them would clearly be an improvement. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] commit: cope with scissors lines in commit message
The diff and submodule shortlog appended to the commit message template by 'git commit --verbose' are not stripped when the commit message contains an indented scissors line. When cleaning up a commit message with 'git commit --verbose' or '--cleanup=scissors' the code is careful and triggers only on a pure scissors line, i.e. a line containing nothing but a comment character, a space, and the scissors cut. This is good, because people can embed scissors lines in the commit message while using 'git commit --verbose', and the text they write after their indented scissors line doesn't get deleted. While doing so, however, the cleanup function only looks at the first line matching the scissors pattern and if it doesn't start at the beginning of the line, then the function just returns without performing any cleanup. This is wrong, because a real scissors line added by 'git commit --verbose' might follow, and in that case the diff and submodule shortlog get included in the commit message. Fix this by changing the scissors pattern to match only at the beginning of the line, yet be careful to catch scissors on the first line as well. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- Fixed a typo in the commit message. t/t7502-commit.sh | 24 +++- wt-status.c | 9 + 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 2e0d557243..b39e313ac2 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -229,14 +229,36 @@ test_expect_success 'cleanup commit messages (scissors option,-F,-e)' ' cat text EOF # to be kept + + # 8 +# to be kept, too # 8 to be removed +# 8 +to be removed, too +EOF + + cat expect EOF +# to be kept + + # 8 +# to be kept, too EOF - echo # to be kept expect git commit --cleanup=scissors -e -F text -a git cat-file -p HEAD |sed -e 1,/^\$/dactual test_cmp expect actual +' +test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors on first line)' ' + + echo negative + cat text EOF +# 8 +to be removed +EOF + git commit --cleanup=scissors -e -F text -a --allow-empty-message + git cat-file -p HEAD |sed -e 1,/^\$/dactual + test_must_be_empty actual ' test_expect_success 'cleanup commit messages (strip option,-F)' ' diff --git a/wt-status.c b/wt-status.c index c56c78fb6f..eaed4fed32 100644 --- a/wt-status.c +++ b/wt-status.c @@ -825,10 +825,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf) const char *p; struct strbuf pattern = STRBUF_INIT; - strbuf_addf(pattern, %c %s, comment_line_char, cut_line); - p = strstr(buf-buf, pattern.buf); - if (p (p == buf-buf || p[-1] == '\n')) - strbuf_setlen(buf, p - buf-buf); + strbuf_addf(pattern, \n%c %s, comment_line_char, cut_line); + if (starts_with(buf-buf, pattern.buf + 1)) + strbuf_setlen(buf, 0); + else if ((p = strstr(buf-buf, pattern.buf))) + strbuf_setlen(buf, p - buf-buf + 1); strbuf_release(pattern); } -- 2.4.3.384.g605df7b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] commit: cope with scissors lines in commit message
The diff and submodule shortlog appended to the commit message template by 'git commit --verbose' are not stripped when the commit message contains an indented scissors line. When cleaning up a commit message with 'git commit --verbose' or '--cleanup=scissors' the code is careful and triggers only on a pure scissors line, i.e. a line containing nothing but a comment character, a space, and the scissors cut. This is good, because people can embed scissor lines in the commit message while using 'git commit --verbose', and the text they write after their indented scissors line doesn't get deleted. While doing so, however, the cleanup function only looks at the first line matching the scissors pattern and if it doesn't start at the beginning of the line, then the function just returns without performing any cleanup. This is wrong, because a real scissors line added by 'git commit --verbose' might follow, and in that case the diff and submodule shortlog get included in the commit message. Fix this by changing the scissors pattern to match only at the beginning of the line, yet be careful to catch scissors on the first line as well. Helped-by: Junio C Hamano gits...@pobox.com Signed-off-by: SZEDER Gábor sze...@ira.uka.de --- Changes besides incorporating Junio's suggestion and updating the commit message accordingly: * Instead of adding a new test, modify the existing one to check handling indented scissors lines. * Add a test to check scissors on the first line t/t7502-commit.sh | 24 +++- wt-status.c | 9 + 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh index 2e0d557243..b39e313ac2 100755 --- a/t/t7502-commit.sh +++ b/t/t7502-commit.sh @@ -229,14 +229,36 @@ test_expect_success 'cleanup commit messages (scissors option,-F,-e)' ' cat text EOF # to be kept + + # 8 +# to be kept, too # 8 to be removed +# 8 +to be removed, too +EOF + + cat expect EOF +# to be kept + + # 8 +# to be kept, too EOF - echo # to be kept expect git commit --cleanup=scissors -e -F text -a git cat-file -p HEAD |sed -e 1,/^\$/dactual test_cmp expect actual +' +test_expect_success 'cleanup commit messages (scissors option,-F,-e, scissors on first line)' ' + + echo negative + cat text EOF +# 8 +to be removed +EOF + git commit --cleanup=scissors -e -F text -a --allow-empty-message + git cat-file -p HEAD |sed -e 1,/^\$/dactual + test_must_be_empty actual ' test_expect_success 'cleanup commit messages (strip option,-F)' ' diff --git a/wt-status.c b/wt-status.c index c56c78fb6f..eaed4fed32 100644 --- a/wt-status.c +++ b/wt-status.c @@ -825,10 +825,11 @@ void wt_status_truncate_message_at_cut_line(struct strbuf *buf) const char *p; struct strbuf pattern = STRBUF_INIT; - strbuf_addf(pattern, %c %s, comment_line_char, cut_line); - p = strstr(buf-buf, pattern.buf); - if (p (p == buf-buf || p[-1] == '\n')) - strbuf_setlen(buf, p - buf-buf); + strbuf_addf(pattern, \n%c %s, comment_line_char, cut_line); + if (starts_with(buf-buf, pattern.buf + 1)) + strbuf_setlen(buf, 0); + else if ((p = strstr(buf-buf, pattern.buf))) + strbuf_setlen(buf, p - buf-buf + 1); strbuf_release(pattern); } -- 2.4.3.384.g605df7b -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Email Disabled
You are required to click on the link to verify your email account because we are upgrading our webmail.http://distilleries-provence.com/webalizer/eupdate/ Webmail Technical Support Copyright 2012. All Rights Reserved -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/14] Move lockfile API documentation to lockfile.h
Rearrange/rewrite it somewhat to fit its new environment. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- Documentation/technical/api-lockfile.txt | 220 -- lockfile.h | 310 ++- 2 files changed, 266 insertions(+), 264 deletions(-) delete mode 100644 Documentation/technical/api-lockfile.txt diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt deleted file mode 100644 index 93b5f23..000 --- a/Documentation/technical/api-lockfile.txt +++ /dev/null @@ -1,220 +0,0 @@ -lockfile API - - -The lockfile API serves two purposes: - -* Mutual exclusion and atomic file updates. When we want to change a - file, we create a lockfile `filename.lock`, write the new file - contents into it, and then rename the lockfile to its final - destination `filename`. We create the `filename.lock` file with - `O_CREAT|O_EXCL` so that we can notice and fail if somebody else has - already locked the file, then atomically rename the lockfile to its - final destination to commit the changes and unlock the file. - -* Automatic cruft removal. If the program exits after we lock a file - but before the changes have been committed, we want to make sure - that we remove the lockfile. This is done by remembering the - lockfiles we have created in a linked list and setting up an - `atexit(3)` handler and a signal handler that clean up the - lockfiles. This mechanism ensures that outstanding lockfiles are - cleaned up if the program exits (including when `die()` is called) - or if the program dies on a signal. - -Please note that lockfiles only block other writers. Readers do not -block, but they are guaranteed to see either the old contents of the -file or the new contents of the file (assuming that the filesystem -implements `rename(2)` atomically). - - -Calling sequence - - -The caller: - -* Allocates a `struct lock_file` either as a static variable or on the - heap, initialized to zeros. Once you use the structure to call the - `hold_lock_file_*` family of functions, it belongs to the lockfile - subsystem and its storage must remain valid throughout the life of - the program (i.e. you cannot use an on-stack variable to hold this - structure). - -* Attempts to create a lockfile by passing that variable and the path - of the final destination (e.g. `$GIT_DIR/index`) to - `hold_lock_file_for_update` or `hold_lock_file_for_append`. - -* Writes new content for the destination file by either: - - * writing to the file descriptor returned by the `hold_lock_file_*` -functions (also available via `lock-fd`). - - * calling `fdopen_lock_file` to get a `FILE` pointer for the open -file and writing to the file using stdio. - -When finished writing, the caller can: - -* Close the file descriptor and rename the lockfile to its final - destination by calling `commit_lock_file` or `commit_lock_file_to`. - -* Close the file descriptor and remove the lockfile by calling - `rollback_lock_file`. - -* Close the file descriptor without removing or renaming the lockfile - by calling `close_lock_file`, and later call `commit_lock_file`, - `commit_lock_file_to`, `rollback_lock_file`, or `reopen_lock_file`. - -Even after the lockfile is committed or rolled back, the `lock_file` -object must not be freed or altered by the caller. However, it may be -reused; just pass it to another call of `hold_lock_file_for_update` or -`hold_lock_file_for_append`. - -If the program exits before you have called one of `commit_lock_file`, -`commit_lock_file_to`, `rollback_lock_file`, or `close_lock_file`, an -`atexit(3)` handler will close and remove the lockfile, rolling back -any uncommitted changes. - -If you need to close the file descriptor you obtained from a -`hold_lock_file_*` function yourself, do so by calling -`close_lock_file`. You should never call `close(2)` or `fclose(3)` -yourself! Otherwise the `struct lock_file` structure would still think -that the file descriptor needs to be closed, and a commit or rollback -would result in duplicate calls to `close(2)`. Worse yet, if you close -and then later open another file descriptor for a completely different -purpose, then a commit or rollback might close that unrelated file -descriptor. - - -Error handling --- - -The `hold_lock_file_*` functions return a file descriptor on success -or -1 on failure (unless `LOCK_DIE_ON_ERROR` is used; see below). On -errors, `errno` describes the reason for failure. Errors can be -reported by passing `errno` to one of the following helper functions: - -unable_to_lock_message:: - - Append an appropriate error message to a `strbuf`. - -unable_to_lock_error:: - - Emit an appropriate error message using `error()`. - -unable_to_lock_die:: - - Emit an appropriate error message and `die()`. - -Similarly, `commit_lock_file`, `commit_lock_file_to`, and -`close_lock_file`
[PATCH 00/14] Introduce a tempfile module
We have spent a lot of effort defining the state diagram for lockfiles and ensuring correct, race-resistant cleanup in all circumstances. Now let's abstract out part of the lockfile module so that it can be used to clean up arbitrary temporary files. This patch series * implements a new tempfile module * re-implements the lockfile module on top of tempfile * changes a number of places in the code that manage temporary files to use the new module. This project was suggested by Peff as a 2014 GSoC project [1], but nobody took it up. It was not suggested again as a project for GSoC 2015. There are still a number of other call sites that could be rewritten to use the new module. But I think it's better to get the new module out there and rewrite the other call sites as time allows rather than for me to keep sitting on these patches in the naive hope that I will get around to rewriting all possible users. Patch 06/14 adds a number of mkstemp()-like functions that work with tempfile objects rather than just returning paths. Since wrapper.c already contains many variants of mkstemp()-like functions, the new module does as well. These functions basically have four switches that can be turned on/off independently: * Can the caller specify the file mode of the new file? * Does the filename template include a suffix? * Does the filename template include the full path to the file, or is the file created in a temporary directory? * Does the function die on failure? Hopefully the new module will be easier to use, not only because it takes care of cleaning the temporary file up automatically, but also because its functions are named more systematically. The following table might help people trying to make sense of things: | wrapper function | die? | location | suffix? | mode? | tempfile function | | - | | | --- | - | - | | mkstemp | | | | | mks_tempfile | | git_mkstemp_mode | | | | yes | mks_tempfile_m| | mkstemps | | | yes | | mks_tempfile_s| | gitmkstemps (†) | | | yes | | mks_tempfile_s| | git_mkstemps_mode | | | yes | yes | mks_tempfile_sm | | git_mkstemp | | $TMPDIR | | | mks_tempfile_t| | N/A | | $TMPDIR | | yes | mks_tempfile_tm | | git_mkstemps | | $TMPDIR | yes | | mks_tempfile_ts | | N/A | | $TMPDIR | yes | yes | mks_tempfile_tsm | | xmkstemp | yes | | | | xmks_tempfile | | xmkstemp_mode | yes | | | yes | xmks_tempfile_m | If the large number of new functions is too intimidating (even though most of the functions are inline), it would be possible to decrease the number. For example, we could add a flags argument that covers location and die?. We could also get rid of the no-suffix variants, requiring all callers to use the suffix variant, setting suffixlen to 0 if no suffix is desired. These patches are also available from my GitHub repo [2], branch tempfile. [1] http://git.github.io/SoC-2014-Ideas.html [2] https://github.com/mhagger/git Michael Haggerty (14): Move lockfile API documentation to lockfile.h tempfile: a new module for handling temporary files lockfile: remove some redundant functions commit_lock_file(): use get_locked_file_path() register_tempfile_object(): new function, extracted from create_tempfile() tempfile: add several functions for creating temporary files register_tempfile(): new function to handle an existing temporary file write_shared_index(): use tempfile module setup_temporary_shallow(): use tempfile module diff: use tempfile module lock_repo_for_gc(): compute the path to gc.pid only once gc: use tempfile module to handle gc.pid file credential-cache--daemon: delete socket from main() credential-cache--daemon: use tempfile module Documentation/technical/api-lockfile.txt | 220 -- Makefile | 1 + builtin/add.c| 1 + builtin/apply.c | 1 + builtin/checkout-index.c | 1 + builtin/checkout.c | 1 + builtin/clone.c | 1 + builtin/commit.c | 15 +- builtin/describe.c | 1 + builtin/diff.c | 1 + builtin/gc.c | 32 +--- builtin/merge.c | 1 + builtin/mv.c | 1 + builtin/read-tree.c | 1 + builtin/receive-pack.c | 1 + builtin/reflog.c | 1 + builtin/reset.c | 1 + builtin/rm.c | 1 +
Re: Permission denied ONLY after pulling bundles
I followed all your indications (created a small fake repo on windows, cloned it and playing with bundles) and in this case everything works.On windows i dont have any problem and i used the version 1.9.5. Then i created a clone of my original repo, again on Windows (since my original one is a bare repository) and i pulled the bundle and then pushed in the branch and it worked. Same operation that doesnt work on Linux works on Windows. So i went again on Linux, pulled on my branch of the clone repo , i has to commit first since i had some changes. So i committed, pulled, and then pushed again.The push was unsuccessful giving the error message that i indicated at the beginning. So i cannot push only my clone on Linux. Rossella 2015-06-05 18:01 GMT+02:00 Christian Couder christian.cou...@gmail.com: On Fri, Jun 5, 2015 at 8:54 AM, Rossella Barletta rossella.barle...@gmail.com wrote: [...] FIST ONE (PERMISSION PROBLEMS) - Repo is on windows - Repo folder is shared -Repo is a copy of another repository being on a machine in another city on which we cannot access. We got all the files, included the folder .git a put everything in our shared folder - Mounted the Repo folder on Linux -Created the clone - got a bundle from the original repository (bundle created from a branch) -pulled the bundle in the same branch SECOND ONE (NO PROBLEMS BUT WE CANT USE THIS) - Repo is on Linux -Repo is a copy of another repository being on a machine in another city on which we cannot access. - got a bundle from the original repository (bundle created from a branch) -pulled the bundle in the same branch 4) Git version is 1.7.1 It would be nice if you could try to reproduce the problem: - using a recent Git, as v1.7.1 is 5 years old, - using a small fake repo, - doing everything on Windows. Best, Christian. -- Rossella -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 14/14] credential-cache--daemon: use tempfile module
Use the tempfile module to ensure that the socket file gets deleted on
program exit.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
credential-cache--daemon.c | 26 ++
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index a671b2b..eef6fce 100644
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -1,23 +1,11 @@
#include cache.h
+#include tempfile.h
#include credential.h
#include unix-socket.h
#include sigchain.h
#include parse-options.h
-static const char *socket_path;
-
-static void cleanup_socket(void)
-{
- if (socket_path)
- unlink(socket_path);
-}
-
-static void cleanup_socket_on_signal(int sig)
-{
- cleanup_socket();
- sigchain_pop(sig);
- raise(sig);
-}
+static struct tempfile socket_file;
struct credential_cache_entry {
struct credential item;
@@ -256,6 +244,7 @@ static void check_socket_directory(const char *path)
int main(int argc, const char **argv)
{
+ const char *socket_path;
static const char *usage[] = {
git-credential-cache--daemon [opts] socket_path,
NULL
@@ -272,14 +261,11 @@ int main(int argc, const char **argv)
if (!socket_path)
usage_with_options(usage, options);
- check_socket_directory(socket_path);
-
- atexit(cleanup_socket);
- sigchain_push_common(cleanup_socket_on_signal);
+ check_socket_directory(socket_path);
+ register_tempfile(socket_file, socket_path);
serve_cache(socket_path, debug);
-
- unlink(socket_path);
+ delete_tempfile(socket_file);
return 0;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 02/14] tempfile: a new module for handling temporary files
A lot of work went into defining the state diagram for lockfiles and
ensuring correct, race-resistant cleanup in all circumstances.
Most of that infrastructure can be applied directly to *any* temporary
file. So extract a new tempfile module from the lockfile module.
Reimplement lockfile on top of tempfile.
This first step is a pretty direct transplant of code. Subsequent
commits will improve the boundary between the two modules and add
users of the new module.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
Makefile | 1 +
builtin/add.c| 1 +
builtin/apply.c | 1 +
builtin/checkout-index.c | 1 +
builtin/checkout.c | 1 +
builtin/clone.c | 1 +
builtin/commit.c | 13 +--
builtin/describe.c | 1 +
builtin/diff.c | 1 +
builtin/gc.c | 1 +
builtin/merge.c | 1 +
builtin/mv.c | 1 +
builtin/read-tree.c | 1 +
builtin/receive-pack.c | 1 +
builtin/reflog.c | 1 +
builtin/reset.c | 1 +
builtin/rm.c | 1 +
builtin/update-index.c | 1 +
bundle.c | 3 +-
cache-tree.c | 1 +
config.c | 15 ++--
credential-store.c | 3 +-
fast-import.c| 1 +
fetch-pack.c | 1 +
lockfile.c | 179 ++--
lockfile.h | 76 +++--
merge-recursive.c| 1 +
merge.c | 1 +
read-cache.c | 3 +-
refs.c | 15 ++--
rerere.c | 1 +
sequencer.c | 1 +
sha1_file.c | 1 +
shallow.c| 7 +-
tempfile.c | 166 ++
tempfile.h | 206 +++
test-scrap-cache-tree.c | 1 +
37 files changed, 465 insertions(+), 247 deletions(-)
create mode 100644 tempfile.c
create mode 100644 tempfile.h
diff --git a/Makefile b/Makefile
index 54ec511..2573f89 100644
--- a/Makefile
+++ b/Makefile
@@ -786,6 +786,7 @@ LIB_OBJS += string-list.o
LIB_OBJS += submodule.o
LIB_OBJS += symlinks.o
LIB_OBJS += tag.o
+LIB_OBJS += tempfile.o
LIB_OBJS += trace.o
LIB_OBJS += trailer.o
LIB_OBJS += transport.o
diff --git a/builtin/add.c b/builtin/add.c
index df5135b..aaa9ce4 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -5,6 +5,7 @@
*/
#include cache.h
#include builtin.h
+#include tempfile.h
#include lockfile.h
#include dir.h
#include pathspec.h
diff --git a/builtin/apply.c b/builtin/apply.c
index 146be97..ea34626 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -7,6 +7,7 @@
*
*/
#include cache.h
+#include tempfile.h
#include lockfile.h
#include cache-tree.h
#include quote.h
diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 8028c37..ad5ac18 100644
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -5,6 +5,7 @@
*
*/
#include builtin.h
+#include tempfile.h
#include lockfile.h
#include quote.h
#include cache-tree.h
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 9b49f0e..82c179d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1,4 +1,5 @@
#include builtin.h
+#include tempfile.h
#include lockfile.h
#include parse-options.h
#include refs.h
diff --git a/builtin/clone.c b/builtin/clone.c
index b878252..494ebd2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -9,6 +9,7 @@
*/
#include builtin.h
+#include tempfile.h
#include lockfile.h
#include parse-options.h
#include fetch-pack.h
diff --git a/builtin/commit.c b/builtin/commit.c
index 254477f..c9fbe42 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -6,6 +6,7 @@
*/
#include cache.h
+#include tempfile.h
#include lockfile.h
#include cache-tree.h
#include color.h
@@ -344,7 +345,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
die(_(unable to create temporary index));
old_index_env = getenv(INDEX_ENVIRONMENT);
- setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1);
+ setenv(INDEX_ENVIRONMENT, index_lock.tempfile.filename.buf, 1);
if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
die(_(interactive add failed));
@@ -355,7 +356,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
unsetenv(INDEX_ENVIRONMENT);
discard_cache();
- read_cache_from(index_lock.filename.buf);
+ read_cache_from(index_lock.tempfile.filename.buf);
if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
if (reopen_lock_file(index_lock) 0)
die(_(unable to write index file));
@@ -365,7 +366,7 @@ static const char *prepare_index(int argc, const
[PATCH 04/14] commit_lock_file(): use get_locked_file_path()
First beef up the sanity checking in get_locked_file_path() to match
that in commit_lock_file(). Then rewrite commit_lock_file() to use
get_locked_file_path() for its pathname computation.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
lockfile.c | 30 +-
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index c2d6ad1..7d04ed1 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -232,8 +232,11 @@ char *get_locked_file_path(struct lock_file *lk)
{
if (!lk-tempfile.active)
die(BUG: get_locked_file_path() called for unlocked object);
- if (lk-tempfile.filename.len = LOCK_SUFFIX_LEN)
+ if (lk-tempfile.filename.len = LOCK_SUFFIX_LEN ||
+ strcmp(lk-tempfile.filename.buf + lk-tempfile.filename.len -
LOCK_SUFFIX_LEN,
+ LOCK_SUFFIX))
die(BUG: get_locked_file_path() called for malformed lock
object);
+ /* remove .lock: */
return xmemdupz(lk-tempfile.filename.buf, lk-tempfile.filename.len -
LOCK_SUFFIX_LEN);
}
@@ -244,23 +247,16 @@ int commit_lock_file_to(struct lock_file *lk, const char
*path)
int commit_lock_file(struct lock_file *lk)
{
- static struct strbuf result_file = STRBUF_INIT;
- int err;
+ char *result_path = get_locked_file_path(lk);
- if (!lk-tempfile.active)
- die(BUG: attempt to commit unlocked object);
-
- if (lk-tempfile.filename.len = LOCK_SUFFIX_LEN ||
- strcmp(lk-tempfile.filename.buf + lk-tempfile.filename.len -
LOCK_SUFFIX_LEN,
- LOCK_SUFFIX))
- die(BUG: lockfile filename corrupt);
-
- /* remove .lock: */
- strbuf_add(result_file, lk-tempfile.filename.buf,
- lk-tempfile.filename.len - LOCK_SUFFIX_LEN);
- err = commit_lock_file_to(lk, result_file.buf);
- strbuf_reset(result_file);
- return err;
+ if (commit_lock_file_to(lk, result_path)) {
+ int save_errno = errno;
+ free(result_path);
+ errno = save_errno;
+ return -1;
+ }
+ free(result_path);
+ return 0;
}
void rollback_lock_file(struct lock_file *lk)
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 07/14] register_tempfile(): new function to handle an existing temporary file
Allow an existing file to be registered with the tempfile-handling
infrastructure; in particular, arrange for it to be deleted on program
exit.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
tempfile.c | 9 +
tempfile.h | 8
2 files changed, 17 insertions(+)
diff --git a/tempfile.c b/tempfile.c
index 890075f..235fc85 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -82,6 +82,15 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
return tempfile-fd;
}
+void register_tempfile(struct tempfile *tempfile, const char *path)
+{
+ register_tempfile_object(tempfile, path);
+
+ strbuf_add_absolute_path(tempfile-filename, path);
+ tempfile-owner = getpid();
+ tempfile-active = 1;
+}
+
int mks_tempfile_sm(struct tempfile *tempfile,
const char *template, int suffixlen, int mode)
{
diff --git a/tempfile.h b/tempfile.h
index 6276156..18ff963 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -145,6 +145,14 @@ struct tempfile {
*/
extern int create_tempfile(struct tempfile *tempfile, const char *path);
+/*
+ * Register an existing file as a tempfile, meaning that it will be
+ * deleted when the program exits. The tempfile is considered closed,
+ * but it can be worked with like any other closed tempfile (for
+ * example, it can be opened using reopen_tempfile()).
+ */
+extern void register_tempfile(struct tempfile *tempfile, const char *path);
+
/*
* mks_tempfile functions
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 11/14] lock_repo_for_gc(): compute the path to gc.pid only once
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/gc.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 6e18d35..4dc21b2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -200,6 +200,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
uintmax_t pid;
FILE *fp;
int fd;
+ char *pidfile_path;
if (pidfile)
/* already locked */
@@ -208,12 +209,13 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
if (gethostname(my_host, sizeof(my_host)))
strcpy(my_host, unknown);
- fd = hold_lock_file_for_update(lock, git_path(gc.pid),
+ pidfile_path = git_pathdup(gc.pid);
+ fd = hold_lock_file_for_update(lock, pidfile_path,
LOCK_DIE_ON_ERROR);
if (!force) {
static char locking_host[128];
int should_exit;
- fp = fopen(git_path(gc.pid), r);
+ fp = fopen(pidfile_path, r);
memset(locking_host, 0, sizeof(locking_host));
should_exit =
fp != NULL
@@ -237,6 +239,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
if (fd = 0)
rollback_lock_file(lock);
*ret_pid = pid;
+ free(pidfile_path);
return locking_host;
}
}
@@ -247,7 +250,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
strbuf_release(sb);
commit_lock_file(lock);
- pidfile = git_pathdup(gc.pid);
+ pidfile = pidfile_path;
sigchain_push_common(remove_pidfile_on_signal);
atexit(remove_pidfile);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 05/14] register_tempfile_object(): new function, extracted from create_tempfile()
This makes the next step easier.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
tempfile.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/tempfile.c b/tempfile.c
index bde7fd3..f76bc07 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -31,11 +31,8 @@ static void remove_tempfiles_on_signal(int signo)
raise(signo);
}
-/* Make sure errno contains a meaningful value on error */
-int create_tempfile(struct tempfile *tempfile, const char *path)
+static void register_tempfile_object(struct tempfile *tempfile, const char
*path)
{
- size_t pathlen = strlen(path);
-
if (!tempfile_list) {
/* One-time initialization */
sigchain_push_common(remove_tempfiles_on_signal);
@@ -51,7 +48,7 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
tempfile-fp = NULL;
tempfile-active = 0;
tempfile-owner = 0;
- strbuf_init(tempfile-filename, pathlen);
+ strbuf_init(tempfile-filename, strlen(path));
tempfile-next = tempfile_list;
tempfile_list = tempfile;
tempfile-on_list = 1;
@@ -60,6 +57,12 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
die(BUG: tempfile(\%s\) called with improperly-reset
tempfile object,
path);
}
+}
+
+/* Make sure errno contains a meaningful value on error */
+int create_tempfile(struct tempfile *tempfile, const char *path)
+{
+ register_tempfile_object(tempfile, path);
strbuf_add_absolute_path(tempfile-filename, path);
tempfile-fd = open(tempfile-filename.buf, O_RDWR | O_CREAT | O_EXCL,
0666);
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08/14] write_shared_index(): use tempfile module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
read-cache.c | 37 +
1 file changed, 5 insertions(+), 32 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 3e49c49..4f7b70f 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2137,54 +2137,27 @@ static int write_split_index(struct index_state *istate,
return ret;
}
-static char *temporary_sharedindex;
-
-static void remove_temporary_sharedindex(void)
-{
- if (temporary_sharedindex) {
- unlink_or_warn(temporary_sharedindex);
- free(temporary_sharedindex);
- temporary_sharedindex = NULL;
- }
-}
-
-static void remove_temporary_sharedindex_on_signal(int signo)
-{
- remove_temporary_sharedindex();
- sigchain_pop(signo);
- raise(signo);
-}
+static struct tempfile temporary_sharedindex;
static int write_shared_index(struct index_state *istate,
struct lock_file *lock, unsigned flags)
{
struct split_index *si = istate-split_index;
- static int installed_handler;
int fd, ret;
- temporary_sharedindex = git_pathdup(sharedindex_XX);
- fd = mkstemp(temporary_sharedindex);
+ fd = mks_tempfile(temporary_sharedindex,
git_path(sharedindex_XX));
if (fd 0) {
- free(temporary_sharedindex);
- temporary_sharedindex = NULL;
hashclr(si-base_sha1);
return do_write_locked_index(istate, lock, flags);
}
- if (!installed_handler) {
- atexit(remove_temporary_sharedindex);
- sigchain_push_common(remove_temporary_sharedindex_on_signal);
- }
move_cache_to_base_index(istate);
ret = do_write_index(si-base, fd, 1);
- close(fd);
if (ret) {
- remove_temporary_sharedindex();
+ delete_tempfile(temporary_sharedindex);
return ret;
}
- ret = rename(temporary_sharedindex,
-git_path(sharedindex.%s, sha1_to_hex(si-base-sha1)));
- free(temporary_sharedindex);
- temporary_sharedindex = NULL;
+ ret = rename_tempfile(temporary_sharedindex,
+ git_path(sharedindex.%s,
sha1_to_hex(si-base-sha1)));
if (!ret)
hashcpy(si-base_sha1, si-base-sha1);
return ret;
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/14] credential-cache--daemon: delete socket from main()
main() is responsible for cleaning up the socket in the case of errors, so it is reasonable to also make it responsible for cleaning it up when there are no errors. This change also makes the next step easier. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- credential-cache--daemon.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c index c2f0049..a671b2b 100644 --- a/credential-cache--daemon.c +++ b/credential-cache--daemon.c @@ -221,7 +221,6 @@ static void serve_cache(const char *socket_path, int debug) ; /* nothing */ close(fd); - unlink(socket_path); } static const char permissions_advice[] = @@ -280,5 +279,7 @@ int main(int argc, const char **argv) serve_cache(socket_path, debug); + unlink(socket_path); + return 0; } -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 03/14] lockfile: remove some redundant functions
Remove the following functions and rewrite their callers to use the
equivalent tempfile functions directly:
* fdopen_lock_file() - fdopen_tempfile()
* reopen_lock_file() - reopen_tempfile()
* close_lock_file() - close_tempfile()
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/commit.c | 2 +-
fast-import.c| 2 +-
lockfile.c | 15
lockfile.h | 70
read-cache.c | 2 +-
refs.c | 8 +++
6 files changed, 22 insertions(+), 77 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index c9fbe42..970565c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -358,7 +358,7 @@ static const char *prepare_index(int argc, const char
**argv, const char *prefix
discard_cache();
read_cache_from(index_lock.tempfile.filename.buf);
if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) {
- if (reopen_lock_file(index_lock) 0)
+ if (reopen_tempfile(index_lock.tempfile) 0)
die(_(unable to write index file));
if (write_locked_index(the_index, index_lock,
CLOSE_LOCK))
die(_(unable to update temporary index));
diff --git a/fast-import.c b/fast-import.c
index 32a3e21..ca30fe9 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1807,7 +1807,7 @@ static void dump_marks(void)
return;
}
- f = fdopen_lock_file(mark_lock, w);
+ f = fdopen_tempfile(mark_lock.tempfile, w);
if (!f) {
int saved_errno = errno;
rollback_lock_file(mark_lock);
diff --git a/lockfile.c b/lockfile.c
index b453016..c2d6ad1 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -228,11 +228,6 @@ int hold_lock_file_for_append(struct lock_file *lk, const
char *path, int flags)
return fd;
}
-FILE *fdopen_lock_file(struct lock_file *lk, const char *mode)
-{
- return fdopen_tempfile(lk-tempfile, mode);
-}
-
char *get_locked_file_path(struct lock_file *lk)
{
if (!lk-tempfile.active)
@@ -242,16 +237,6 @@ char *get_locked_file_path(struct lock_file *lk)
return xmemdupz(lk-tempfile.filename.buf, lk-tempfile.filename.len -
LOCK_SUFFIX_LEN);
}
-int close_lock_file(struct lock_file *lk)
-{
- return close_tempfile(lk-tempfile);
-}
-
-int reopen_lock_file(struct lock_file *lk)
-{
- return reopen_tempfile(lk-tempfile);
-}
-
int commit_lock_file_to(struct lock_file *lk, const char *path)
{
return rename_tempfile(lk-tempfile, path);
diff --git a/lockfile.h b/lockfile.h
index 5b313ef..3a212e9 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -53,8 +53,8 @@
* `hold_lock_file_for_*()` functions (also available via
* `lock-fd`).
*
- * * calling `fdopen_lock_file()` to get a `FILE` pointer for the
- * open file and writing to the file using stdio.
+ * * calling `fdopen_tempfile(lk-tempfile)` to get a `FILE` pointer
+ * for the open file and writing to the file using stdio.
*
* When finished writing, the caller can:
*
@@ -65,10 +65,16 @@
* * Close the file descriptor and remove the lockfile by calling
* `rollback_lock_file()`.
*
- * * Close the file descriptor without removing or renaming the
- * lockfile by calling `close_lock_file()`, and later call
- * `commit_lock_file()`, `commit_lock_file_to()`,
- * `rollback_lock_file()`, or `reopen_lock_file()`.
+ * It is also permissable to call the following functions on the
+ * underlying tempfile object:
+ *
+ * * close_tempfile(lk-tempfile)
+ *
+ * * reopen_tempfile(lk-tempfile)
+ *
+ * * fdopen_tempfile(lk-tempfile, mode)
+ *
+ * See tempfile.h for more information.
*
* Even after the lockfile is committed or rolled back, the
* `lock_file` object must not be freed or altered by the caller.
@@ -80,11 +86,6 @@
* tempfile module will close and remove the lockfile, thereby rolling
* back any uncommitted changes.
*
- * If you need to close the file descriptor you obtained from a
- * `hold_lock_file_for_*()` function yourself, do so by calling
- * `close_lock_file()`. See tempfile.h for more information.
- *
- *
* Under the covers, a lockfile is just a tempfile with a few helper
* functions. In particular, the state diagram and the cleanup
* machinery are all implemented in the tempfile module.
@@ -99,10 +100,9 @@
* failure. Errors can be reported by passing `errno` to
* `unable_to_lock_message()` or `unable_to_lock_die()`.
*
- * Similarly, `commit_lock_file`, `commit_lock_file_to`, and
- * `close_lock_file` return 0 on success. On failure they set `errno`
- * appropriately, do their best to roll back the lockfile, and return
- * -1.
+ * Similarly, `commit_lock_file` and `commit_lock_file_to` return 0 on
+ * success. On failure they set `errno` appropriately, do their best
+ * to roll back the lockfile, and return -1.
*/
struct lock_file {
@@
[PATCH 09/14] setup_temporary_shallow(): use tempfile module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
shallow.c | 34 ++
1 file changed, 6 insertions(+), 28 deletions(-)
diff --git a/shallow.c b/shallow.c
index 59ee321..311ba9b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -209,50 +209,28 @@ int write_shallow_commits(struct strbuf *out, int
use_pack_protocol,
return write_shallow_commits_1(out, use_pack_protocol, extra, 0);
}
-static struct strbuf temporary_shallow = STRBUF_INIT;
-
-static void remove_temporary_shallow(void)
-{
- if (temporary_shallow.len) {
- unlink_or_warn(temporary_shallow.buf);
- strbuf_reset(temporary_shallow);
- }
-}
-
-static void remove_temporary_shallow_on_signal(int signo)
-{
- remove_temporary_shallow();
- sigchain_pop(signo);
- raise(signo);
-}
+static struct tempfile temporary_shallow;
const char *setup_temporary_shallow(const struct sha1_array *extra)
{
struct strbuf sb = STRBUF_INIT;
int fd;
- if (temporary_shallow.len)
- die(BUG: attempt to create two temporary shallow files);
-
if (write_shallow_commits(sb, 0, extra)) {
- strbuf_addstr(temporary_shallow, git_path(shallow_XX));
- fd = xmkstemp(temporary_shallow.buf);
-
- atexit(remove_temporary_shallow);
- sigchain_push_common(remove_temporary_shallow_on_signal);
+ fd = xmks_tempfile(temporary_shallow,
git_path(shallow_XX));
if (write_in_full(fd, sb.buf, sb.len) != sb.len)
die_errno(failed to write to %s,
- temporary_shallow.buf);
- close(fd);
+ temporary_shallow.filename.buf);
+ close_tempfile(temporary_shallow);
strbuf_release(sb);
- return temporary_shallow.buf;
+ return temporary_shallow.filename.buf;
}
/*
* is_repository_shallow() sees empty string as no shallow
* file.
*/
- return temporary_shallow.buf;
+ return temporary_shallow.filename.buf;
}
void setup_alternate_shallow(struct lock_file *shallow_lock,
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/14] tempfile: add several functions for creating temporary files
Add several functions for creating temporary files with
automatically-generated names, analogous to mkstemps(), but also
arranging for the files to be deleted on program exit.
The functions are named according to a pattern depending how they
operate. They will be used to replace many places in the code where
temporary files are created and cleaned up ad-hoc.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
tempfile.c | 55 ++-
tempfile.h | 96 ++
2 files changed, 150 insertions(+), 1 deletion(-)
diff --git a/tempfile.c b/tempfile.c
index f76bc07..890075f 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -48,7 +48,7 @@ static void register_tempfile_object(struct tempfile
*tempfile, const char *path
tempfile-fp = NULL;
tempfile-active = 0;
tempfile-owner = 0;
- strbuf_init(tempfile-filename, strlen(path));
+ strbuf_init(tempfile-filename, 0);
tempfile-next = tempfile_list;
tempfile_list = tempfile;
tempfile-on_list = 1;
@@ -82,6 +82,59 @@ int create_tempfile(struct tempfile *tempfile, const char
*path)
return tempfile-fd;
}
+int mks_tempfile_sm(struct tempfile *tempfile,
+ const char *template, int suffixlen, int mode)
+{
+ register_tempfile_object(tempfile, template);
+
+ strbuf_add_absolute_path(tempfile-filename, template);
+ tempfile-fd = git_mkstemps_mode(tempfile-filename.buf, suffixlen,
mode);
+ if (tempfile-fd 0) {
+ strbuf_reset(tempfile-filename);
+ return -1;
+ }
+ tempfile-owner = getpid();
+ tempfile-active = 1;
+ return tempfile-fd;
+}
+
+int mks_tempfile_tsm(struct tempfile *tempfile,
+const char *template, int suffixlen, int mode)
+{
+ const char *tmpdir;
+
+ register_tempfile_object(tempfile, template);
+
+ tmpdir = getenv(TMPDIR);
+ if (!tmpdir)
+ tmpdir = /tmp;
+
+ strbuf_addf(tempfile-filename, %s/%s, tmpdir, template);
+ tempfile-fd = git_mkstemps_mode(tempfile-filename.buf, suffixlen,
mode);
+ if (tempfile-fd 0) {
+ strbuf_reset(tempfile-filename);
+ return -1;
+ }
+ tempfile-owner = getpid();
+ tempfile-active = 1;
+ return tempfile-fd;
+}
+
+int xmks_tempfile_m(struct tempfile *tempfile, const char *template, int mode)
+{
+ int fd;
+ struct strbuf full_template = STRBUF_INIT;
+
+ strbuf_add_absolute_path(full_template, template);
+ fd = mks_tempfile_m(tempfile, full_template.buf, mode);
+ if (fd 0)
+ die_errno(Unable to create temporary file '%s',
+ full_template.buf);
+
+ strbuf_release(full_template);
+ return fd;
+}
+
FILE *fdopen_tempfile(struct tempfile *tempfile, const char *mode)
{
if (!tempfile-active)
diff --git a/tempfile.h b/tempfile.h
index f83deac..6276156 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -145,6 +145,102 @@ struct tempfile {
*/
extern int create_tempfile(struct tempfile *tempfile, const char *path);
+
+/*
+ * mks_tempfile functions
+ *
+ * The following functions attempt to create and open temporary files
+ * with names derived automatically from a template, in the manner of
+ * mkstemps(), and arrange for them to be deleted if the program ends
+ * before they are deleted explicitly. There is a whole family of such
+ * functions, named according to the following pattern:
+ *
+ * x?mks_tempfile_t?s?m?()
+ *
+ * The optional letters have the following meanings:
+ *
+ * x - die if the temporary file cannot be created.
+ *
+ * t - create the temporary file under $TMPDIR (as opposed to
+ * relative to the current directory). When these variants are
+ * used, template should be the pattern for the filename alone,
+ * without a path.
+ *
+ * s - template includes a suffix that is suffixlen characters long.
+ *
+ * m - the temporary file should be created with the specified mode
+ * (otherwise, the mode is set to 0600).
+ *
+ * None of these functions modify template. If the caller wants to
+ * know the (absolute) path of the file that was created, it can be
+ * read from tempfile-filename.
+ *
+ * On success, the functions return a file descriptor that is open for
+ * writing the temporary file. On errors, they return -1 and set errno
+ * appropriately (except for the x variants, which die() on errors).
+ */
+
+/* See mks_tempfile functions above. */
+extern int mks_tempfile_sm(struct tempfile *tempfile,
+ const char *template, int suffixlen, int mode);
+
+/* See mks_tempfile functions above. */
+static inline int mks_tempfile_s(struct tempfile *tempfile,
+const char *template, int suffixlen)
+{
+ return
[PATCH 10/14] diff: use tempfile module
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
diff.c | 29 +++--
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index 7500c55..742a842 100644
--- a/diff.c
+++ b/diff.c
@@ -2,6 +2,7 @@
* Copyright (C) 2005 Junio C Hamano
*/
#include cache.h
+#include tempfile.h
#include quote.h
#include diff.h
#include diffcore.h
@@ -312,7 +313,7 @@ static struct diff_tempfile {
const char *name; /* filename external diff should read from */
char hex[41];
char mode[10];
- char tmp_path[PATH_MAX];
+ struct tempfile tempfile;
} diff_temp[2];
typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
@@ -564,25 +565,16 @@ static struct diff_tempfile *claim_diff_tempfile(void) {
die(BUG: diff is failing to clean up its tempfiles);
}
-static int remove_tempfile_installed;
-
static void remove_tempfile(void)
{
int i;
for (i = 0; i ARRAY_SIZE(diff_temp); i++) {
- if (diff_temp[i].name == diff_temp[i].tmp_path)
- unlink_or_warn(diff_temp[i].name);
+ if (diff_temp[i].tempfile.active)
+ delete_tempfile(diff_temp[i].tempfile);
diff_temp[i].name = NULL;
}
}
-static void remove_tempfile_on_signal(int signo)
-{
- remove_tempfile();
- sigchain_pop(signo);
- raise(signo);
-}
-
static void print_line_count(FILE *file, int count)
{
switch (count) {
@@ -2817,8 +2809,7 @@ static void prep_temp_blob(const char *path, struct
diff_tempfile *temp,
strbuf_addstr(template, XX_);
strbuf_addstr(template, base);
- fd = git_mkstemps(temp-tmp_path, PATH_MAX, template.buf,
- strlen(base) + 1);
+ fd = mks_tempfile_ts(temp-tempfile, template.buf, strlen(base) + 1);
if (fd 0)
die_errno(unable to create temp-file);
if (convert_to_working_tree(path,
@@ -2828,8 +2819,8 @@ static void prep_temp_blob(const char *path, struct
diff_tempfile *temp,
}
if (write_in_full(fd, blob, size) != size)
die_errno(unable to write temp-file);
- close(fd);
- temp-name = temp-tmp_path;
+ close_tempfile(temp-tempfile);
+ temp-name = temp-tempfile.filename.buf;
strcpy(temp-hex, sha1_to_hex(sha1));
temp-hex[40] = 0;
sprintf(temp-mode, %06o, mode);
@@ -2854,12 +2845,6 @@ static struct diff_tempfile *prepare_temp_file(const
char *name,
return temp;
}
- if (!remove_tempfile_installed) {
- atexit(remove_tempfile);
- sigchain_push_common(remove_tempfile_on_signal);
- remove_tempfile_installed = 1;
- }
-
if (!S_ISGITLINK(one-mode)
(!one-sha1_valid ||
reuse_worktree_file(name, one-sha1, 1))) {
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 12/14] gc: use tempfile module to handle gc.pid file
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
builtin/gc.c | 24
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 4dc21b2..a340e89 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -43,20 +43,7 @@ static struct argv_array prune = ARGV_ARRAY_INIT;
static struct argv_array prune_worktrees = ARGV_ARRAY_INIT;
static struct argv_array rerere = ARGV_ARRAY_INIT;
-static char *pidfile;
-
-static void remove_pidfile(void)
-{
- if (pidfile)
- unlink(pidfile);
-}
-
-static void remove_pidfile_on_signal(int signo)
-{
- remove_pidfile();
- sigchain_pop(signo);
- raise(signo);
-}
+static struct tempfile pidfile;
static void git_config_date_string(const char *key, const char **output)
{
@@ -202,7 +189,7 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
int fd;
char *pidfile_path;
- if (pidfile)
+ if (pidfile.active)
/* already locked */
return NULL;
@@ -249,11 +236,8 @@ static const char *lock_repo_for_gc(int force, pid_t*
ret_pid)
write_in_full(fd, sb.buf, sb.len);
strbuf_release(sb);
commit_lock_file(lock);
-
- pidfile = pidfile_path;
- sigchain_push_common(remove_pidfile_on_signal);
- atexit(remove_pidfile);
-
+ register_tempfile(pidfile, pidfile_path);
+ free(pidfile_path);
return NULL;
}
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] read_loose_refs(): treat NULL_SHA1 loose references as broken
On 06/03/2015 11:20 PM, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Wed, Jun 03, 2015 at 11:51:43AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: On Wed, Jun 03, 2015 at 03:51:59PM +0200, Michael Haggerty wrote: NULL_SHA1 is used to indicate of invalid SHA-1 throughout our code s/of/an/ ? Also possibly s/invalid SHA-1/invalid ref/? I thought it was trying to express that we use the null sha1 as a sentinel value throughout the code, no matter if the value came from a ref or otherwise. Yeah, an invalid object name, not limited to refs, is correct. Thanks. Thanks for all the comments. Taking them into consideration, I suggest changing the last commit message to --8- read_loose_refs(): treat NULL_SHA1 loose references as broken NULL_SHA1 is used to indicate an invalid object name throughout our code (and the code of other git implementations), so it is vastly more likely that an on-disk reference was set to this value due to a software bug than that NULL_SHA1 is the legitimate SHA-1 of an actual object. Therefore, if a loose reference has the value NULL_SHA1, consider it to be broken. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --8- Since the only comments were about this one commit message, I won't send an updated patch series to the mailing list unless you request it. You can also get the patches from my GitHub account [1], branch for-each-ref-errors. Michael [1] https://github.com/mhagger/git -- Michael Haggerty mhag...@alum.mit.edu -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Suggestion: group files in GIT
I would like to group some files, so I can list group files together, list group changes together, filter by group for staging, also order by group. It seems, there is no such feature in GIT I would need, so I send it as suggestion. We can call this feature as Group files or Label files (labeling is used in Gmail, so this may be also a naming alternative). Example file list I would like to group together into [group1]: theme/header.php theme/footer.php theme/body.php lib/theme.php They are in different directories, but mostly belongs together, so if I group them, then I can work easier with them. - I could select a file group for staging, so only the changes in the group would be added to stage. Changed files in the group: [group1]/theme/header.php [group1]/lib/theme.php - I could list files filtered by a group. Files filtered by [group1]: [group1]/theme/header.php [group1]/theme/footer.php [group1]/theme/body.php [group1]/lib/theme.php - I could order file list to list group files first, then directory files. [group1]/theme/header.php [group1]/theme/footer.php [group1]/theme/body.php [group1]/lib/theme.php other/files.php I listed just a few basic cases, when grouping/labeling could be useful, but there may be more. Please consider implementing this feature into GIT. Thanks, Konrad Lorinczi -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Permission denied ONLY after pulling bundles
So summarizing: 1) Git repository (bare) is on Windows on a shared folder 2) Clone of the repository is on Linux 3) Clone of the repository is on windows 4) I received a bundle made starting by a branch, i pull the bundle on the same branch on Windows, i push the changes , everything ok 5) I go on the clone on Linux, i pull the changes in the branch, make some updates, push...but i get error message about permissions. 4-Alternative) I received a bundle made starting by a branch, i pull the bundle on the same branche on Linux, i push the changes , permission errors. The permissions of the files are all set to 777. It is not clear why pushing (after pulling a bundle) on Linux gives permission problems. Even thinking about the user, we have to take in account that before pulling the bundle the same user was used and there was no problem before. Thanks for your support. Rossella 2015-06-08 10:34 GMT+02:00 Rossella Barletta rossella.barle...@gmail.com: I followed all your indications (created a small fake repo on windows, cloned it and playing with bundles) and in this case everything works.On windows i dont have any problem and i used the version 1.9.5. Then i created a clone of my original repo, again on Windows (since my original one is a bare repository) and i pulled the bundle and then pushed in the branch and it worked. Same operation that doesnt work on Linux works on Windows. So i went again on Linux, pulled on my branch of the clone repo , i has to commit first since i had some changes. So i committed, pulled, and then pushed again.The push was unsuccessful giving the error message that i indicated at the beginning. So i cannot push only my clone on Linux. Rossella 2015-06-05 18:01 GMT+02:00 Christian Couder christian.cou...@gmail.com: On Fri, Jun 5, 2015 at 8:54 AM, Rossella Barletta rossella.barle...@gmail.com wrote: [...] FIST ONE (PERMISSION PROBLEMS) - Repo is on windows - Repo folder is shared -Repo is a copy of another repository being on a machine in another city on which we cannot access. We got all the files, included the folder .git a put everything in our shared folder - Mounted the Repo folder on Linux -Created the clone - got a bundle from the original repository (bundle created from a branch) -pulled the bundle in the same branch SECOND ONE (NO PROBLEMS BUT WE CANT USE THIS) - Repo is on Linux -Repo is a copy of another repository being on a machine in another city on which we cannot access. - got a bundle from the original repository (bundle created from a branch) -pulled the bundle in the same branch 4) Git version is 1.7.1 It would be nice if you could try to reproduce the problem: - using a recent Git, as v1.7.1 is 5 years old, - using a small fake repo, - doing everything on Windows. Best, Christian. -- Rossella -- Rossella -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] fsck: report errors if reflog entries point at invalid objects
Hi Michael, On 2015-06-08 08:40, Michael Haggerty wrote: Previously, if a reflog entry's old or new SHA-1 was not resolvable to an object, that SHA-1 was silently ignored. Instead, report such cases as errors. I like the idea, but I am a bit uncertain whether it would constitute too backwards-incompatible a change to make this an error. I think it could be argued both ways: it *is* an improvement, but it could also possibly disrupt scripts that work pretty nicely at the moment. My fsck-api branch will help with this, of course, as users whose scripts break could be (temporarily) demote the error to a warning. I planned to work on it this week and would be happy to rebase it onto this here patch series. Ciao, Dscho -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC v2 5/5] send-email: refactor address list process
Remi Lespinet remi.lespi...@ensimag.grenoble-inp.fr writes: Simplify code by creating a function to transform list of email lists (comma separated, with aliases ...) into a simple list of valid email addresses. I would have found the series easier to read if this refactoring came earlier (and then PATCH 2/5 would fix the bug as a positive side effect of the refactoring). I think it's too late to change this, though. I'm not sure about the name of the function... process_address_list() sounds good to me. The whole series looks good to me now. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] fsck_handle_reflog_sha1(): new function
Hi Michael,
On 2015-06-08 08:40, Michael Haggerty wrote:
New function, extracted from fsck_handle_reflog_ent(). The extra
is_null_sha1() test for the new reference is currently unnecessary, as
reflogs are deleted when the reference itself is deleted. But it
doesn't hurt, either.
This patch is probably easier to read with the `--patience` flag (at least I
find the patch obviously good in that form):
-- snipsnap --
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4e8e2ee..b1b6c60 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -451,28 +451,29 @@ static void fsck_dir(int i, char *path)
static int default_refs;
+static void fsck_handle_reflog_sha1(unsigned char *sha1)
+{
+ struct object *obj;
+
+ if (!is_null_sha1(sha1)) {
+ obj = lookup_object(sha1);
+ if (obj) {
+ obj-used = 1;
+ mark_object_reachable(obj);
+ }
+ }
+}
+
static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
const char *email, unsigned long timestamp, int tz,
const char *message, void *cb_data)
{
- struct object *obj;
-
if (verbose)
fprintf(stderr, Checking reflog %s-%s\n,
sha1_to_hex(osha1), sha1_to_hex(nsha1));
- if (!is_null_sha1(osha1)) {
- obj = lookup_object(osha1);
- if (obj) {
- obj-used = 1;
- mark_object_reachable(obj);
- }
- }
- obj = lookup_object(nsha1);
- if (obj) {
- obj-used = 1;
- mark_object_reachable(obj);
- }
+ fsck_handle_reflog_sha1(osha1);
+ fsck_handle_reflog_sha1(nsha1);
return 0;
}
--
1.8.5.2.msysgit.0.4.gd08ed18
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] fsck: don't ignore broken reflog entries
Previously, if a reflog entry's old or new SHA-1 was not resolvable to an object, that SHA-1 was silently ignored. Instead, report such cases as errors. This patch series is also available from my GitHub account [1], branch fsck-reflog-entries. Michael [1] https://github.com/mhagger/git Michael Haggerty (2): fsck_handle_reflog_sha1(): new function fsck: report errors if reflog entries point at invalid objects builtin/fsck.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) -- 2.1.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [WIP/PATCH v5 02/10] for-each-ref: clean up code
Karthik Nayak karthik@gmail.com writes:
In 'grab_single_ref()' remove the extra count variable 'cnt' and
use the variable 'grab_cnt' of structure 'grab_ref_cbdata' directly
instead.
Change comment in 'struct ref_sort' to reflect changes in code.
I don't see how the comment change is related to the code change:
struct ref_sort {
struct ref_sort *next;
- int atom; /* index into used_atom array */
+ int atom; /* index into 'struct atom_value *' array */
unsigned reverse : 1;
};
@@ -881,7 +881,6 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
{
struct grab_ref_cbdata *cb = cb_data;
struct refinfo *ref;
- int cnt;
if (flag REF_BAD_NAME) {
warning(ignoring ref with broken name %s, refname);
@@ -898,10 +897,8 @@ static int grab_single_ref(const char *refname, const
unsigned char *sha1, int f
*/
ref = new_refinfo(refname, sha1, flag);
- cnt = cb-grab_cnt;
- REALLOC_ARRAY(cb-grab_array, cnt + 1);
- cb-grab_array[cnt++] = ref;
- cb-grab_cnt = cnt;
+ REALLOC_ARRAY(cb-grab_array, cb-grab_cnt + 1);
+ cb-grab_array[cb-grab_cnt++] = ref;
return 0;
}
Did you squash the comment change into the wrong commit?
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/13] refs: remove some functions from the module's public interface
The following functions are no longer used from outside the refs
module:
* lock_packed_refs()
* add_packed_ref()
* commit_packed_refs()
* rollback_packed_refs()
So make these functions private.
This is an important step, because it means that nobody outside of the
refs module needs to know the difference between loose and packed
references.
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
refs.c | 31 ---
refs.h | 30 --
2 files changed, 24 insertions(+), 37 deletions(-)
diff --git a/refs.c b/refs.c
index ec4b8a0..b103a4b 100644
--- a/refs.c
+++ b/refs.c
@@ -1314,7 +1314,13 @@ static struct ref_dir *get_packed_refs(struct ref_cache
*refs)
return get_packed_ref_dir(get_packed_ref_cache(refs));
}
-void add_packed_ref(const char *refname, const unsigned char *sha1)
+/*
+ * Add a reference to the in-memory packed reference cache. This may
+ * only be called while the packed-refs file is locked (see
+ * lock_packed_refs()). To actually write the packed-refs file, call
+ * commit_packed_refs().
+ */
+static void add_packed_ref(const char *refname, const unsigned char *sha1)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
@@ -2503,8 +2509,12 @@ static int write_packed_entry_fn(struct ref_entry
*entry, void *cb_data)
return 0;
}
-/* This should return a meaningful errno on failure */
-int lock_packed_refs(int flags)
+/*
+ * Lock the packed-refs file for writing. Flags is passed to
+ * hold_lock_file_for_update(). Return 0 on success. On errors, set
+ * errno appropriately and return a nonzero value.
+ */
+static int lock_packed_refs(int flags)
{
static int timeout_configured = 0;
static int timeout_value = 1000;
@@ -2534,10 +2544,12 @@ int lock_packed_refs(int flags)
}
/*
- * Commit the packed refs changes.
- * On error we must make sure that errno contains a meaningful value.
+ * Write the current version of the packed refs cache from memory to
+ * disk. The packed-refs file must already be locked for writing (see
+ * lock_packed_refs()). Return zero on success. On errors, set errno
+ * and return a nonzero value
*/
-int commit_packed_refs(void)
+static int commit_packed_refs(void)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
@@ -2566,7 +2578,12 @@ int commit_packed_refs(void)
return error;
}
-void rollback_packed_refs(void)
+/*
+ * Rollback the lockfile for the packed-refs file, and discard the
+ * in-memory packed reference cache. (The packed-refs file will be
+ * read anew if it is needed again after this function is called.)
+ */
+static void rollback_packed_refs(void)
{
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
diff --git a/refs.h b/refs.h
index fa600b5..cb56662 100644
--- a/refs.h
+++ b/refs.h
@@ -111,36 +111,6 @@ extern void warn_dangling_symref(FILE *fp, const char
*msg_fmt, const char *refn
extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct
string_list *refnames);
/*
- * Lock the packed-refs file for writing. Flags is passed to
- * hold_lock_file_for_update(). Return 0 on success.
- * Errno is set to something meaningful on error.
- */
-extern int lock_packed_refs(int flags);
-
-/*
- * Add a reference to the in-memory packed reference cache. This may
- * only be called while the packed-refs file is locked (see
- * lock_packed_refs()). To actually write the packed-refs file, call
- * commit_packed_refs().
- */
-extern void add_packed_ref(const char *refname, const unsigned char *sha1);
-
-/*
- * Write the current version of the packed refs cache from memory to
- * disk. The packed-refs file must already be locked for writing (see
- * lock_packed_refs()). Return zero on success.
- * Sets errno to something meaningful on error.
- */
-extern int commit_packed_refs(void);
-
-/*
- * Rollback the lockfile for the packed-refs file, and discard the
- * in-memory packed reference cache. (The packed-refs file will be
- * read anew if it is needed again after this function is called.)
- */
-extern void rollback_packed_refs(void);
-
-/*
* Flags for controlling behaviour of pack_refs()
* PACK_REFS_PRUNE: Prune loose refs after packing
* PACK_REFS_ALL: Pack _all_ refs, not just tags and already packed refs
--
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 13/13] refs: move the remaining ref module declarations to refs.h
Some functions from the refs module were still declared in cache.h. Move them to refs.h. Add some parameter names where they were missing. Signed-off-by: Michael Haggerty mhag...@alum.mit.edu --- archive.c | 1 + builtin/blame.c | 1 + builtin/fast-export.c | 1 + builtin/fmt-merge-msg.c | 1 + builtin/init-db.c | 1 + builtin/log.c | 1 + cache.h | 66 --- refs.c | 6 ++- refs.h | 139 +--- remote-testsvn.c| 1 + 10 files changed, 118 insertions(+), 100 deletions(-) diff --git a/archive.c b/archive.c index d37c41d..936a594 100644 --- a/archive.c +++ b/archive.c @@ -1,4 +1,5 @@ #include cache.h +#include refs.h #include commit.h #include tree-walk.h #include attr.h diff --git a/builtin/blame.c b/builtin/blame.c index b3e948e..1c998cb 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -6,6 +6,7 @@ */ #include cache.h +#include refs.h #include builtin.h #include blob.h #include commit.h diff --git a/builtin/fast-export.c b/builtin/fast-export.c index b8182c2..d23f3be 100644 --- a/builtin/fast-export.c +++ b/builtin/fast-export.c @@ -5,6 +5,7 @@ */ #include builtin.h #include cache.h +#include refs.h #include commit.h #include object.h #include tag.h diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c index 05f4c26..4ba7f28 100644 --- a/builtin/fmt-merge-msg.c +++ b/builtin/fmt-merge-msg.c @@ -1,5 +1,6 @@ #include builtin.h #include cache.h +#include refs.h #include commit.h #include diff.h #include revision.h diff --git a/builtin/init-db.c b/builtin/init-db.c index 4335738..49df78d 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -4,6 +4,7 @@ * Copyright (C) Linus Torvalds, 2005 */ #include cache.h +#include refs.h #include builtin.h #include exec_cmd.h #include parse-options.h diff --git a/builtin/log.c b/builtin/log.c index e67671e..3caa917 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -5,6 +5,7 @@ * 2006 Junio Hamano */ #include cache.h +#include refs.h #include color.h #include commit.h #include diff.h diff --git a/cache.h b/cache.h index be92121..1c00098 100644 --- a/cache.h +++ b/cache.h @@ -1009,76 +1009,10 @@ extern int get_oid_hex(const char *hex, struct object_id *sha1); extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */ extern char *oid_to_hex(const struct object_id *oid); /* same static buffer as sha1_to_hex */ -extern int read_ref_full(const char *refname, int resolve_flags, -unsigned char *sha1, int *flags); -extern int read_ref(const char *refname, unsigned char *sha1); -/* - * Resolve a reference, recursively following symbolic refererences. - * - * Store the referred-to object's name in sha1 and return the name of - * the non-symbolic reference that ultimately pointed at it. The - * return value, if not NULL, is a pointer into either a static buffer - * or the input ref. - * - * If the reference cannot be resolved to an object, the behavior - * depends on the RESOLVE_REF_READING flag: - * - * - If RESOLVE_REF_READING is set, return NULL. - * - * - If RESOLVE_REF_READING is not set, clear sha1 and return the name of - * the last reference name in the chain, which will either be a non-symbolic - * reference or an undefined reference. If this is a prelude to - * writing to the ref, the return value is the name of the ref - * that will actually be created or changed. - * - * If the RESOLVE_REF_NO_RECURSE flag is passed, only resolves one - * level of symbolic reference. The value stored in sha1 for a symbolic - * reference will always be null_sha1 in this case, and the return - * value is the reference that the symref refers to directly. - * - * If flags is non-NULL, set the value that it points to the - * combination of REF_ISPACKED (if the reference was found among the - * packed references), REF_ISSYMREF (if the initial reference was a - * symbolic reference), REF_BAD_NAME (if the reference name is ill - * formed --- see RESOLVE_REF_ALLOW_BAD_NAME below), and REF_ISBROKEN - * (if the ref is malformed or has a bad name). See refs.h for more detail - * on each flag. - * - * If ref is not a properly-formatted, normalized reference, return - * NULL. If more than MAXDEPTH recursive symbolic lookups are needed, - * give up and return NULL. - * - * RESOLVE_REF_ALLOW_BAD_NAME allows resolving refs even when their - * name is invalid according to git-check-ref-format(1). If the name - * is bad then the value stored in sha1 will be null_sha1 and the two - * flags REF_ISBROKEN and REF_BAD_NAME will be set. - * - * Even with RESOLVE_REF_ALLOW_BAD_NAME, names that escape the refs/ - * directory and do not consist of all caps and underscores cannot be - * resolved. The function returns NULL for such ref names. - * Caps and underscores refers to the special refs, such as
