* Re: [PATCH] real_path: make real_path thread-safe
From: Junio C Hamano @ 2016-12-06 23:44 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, sbeller, peff, jacob.keller
In-Reply-To: <1480964316-99305-2-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> +/* removes the last path component from 'path' except if 'path' is root */
> +static void strip_last_component(struct strbuf *path)
> +{
> + if (path->len > 1) {
> + char *last_slash = find_last_dir_sep(path->buf);
> + strbuf_setlen(path, last_slash - path->buf);
> + }
> +}
You use find_last_dir_sep() which takes care of "Windows uses
backslash" issue. Is this function expected to be fed something
like "C:\My Files\foo.txt" and more importantly "C:\My Files"? Or
is that handled by a lot higher level up in the callchain? I am
reacting the comparison of path->len and 1 here.
Also is the input expected to be normalized? Are we expected to be
fed something like "/a//b/./c/../d/e" and react sensibly, or is that
handled by a lot higher level up in the callchain?
> +/* gets the next component in 'remaining' and places it in 'next' */
> +static void get_next_component(struct strbuf *next, struct strbuf *remaining)
> +{
> + char *start = NULL;
> + char *end = NULL;
> +
> + strbuf_reset(next);
> +
> + /* look for the next component */
> + /* Skip sequences of multiple path-separators */
> + for (start = remaining->buf; is_dir_sep(*start); start++)
> + /* nothing */;
Style:
; /* nothing */
> + /* Find end of the path component */
> + for (end = start; *end && !is_dir_sep(*end); end++)
> + /* nothing */;
> +
> + strbuf_add(next, start, end - start);
OK, so this was given "///foo/bar" in "remaining" and appended
'foo/' to "next". I.e. deduping of slashes is handled here.
POSIX cares about treating "//" at the very beginning of the path
specially. Is that supposed to be handled here, or by a lot higher
level up in the callchain?
> + /* remove the component from 'remaining' */
> + strbuf_remove(remaining, 0, end - remaining->buf);
> +}
> +
> /* We allow "recursive" symbolic links. Only within reason, though. */
> -#define MAXDEPTH 5
> +#define MAXSYMLINKS 5
>
> /*
> * Return the real path (i.e., absolute path, with symlinks resolved
> @@ -21,7 +51,6 @@ int is_directory(const char *path)
> * absolute_path().) The return value is a pointer to a static
> * buffer.
> *
> * The directory part of path (i.e., everything up to the last
> * dir_sep) must denote a valid, existing directory, but the last
> * component need not exist. If die_on_error is set, then die with an
> @@ -33,22 +62,16 @@ int is_directory(const char *path)
> */
> static const char *real_path_internal(const char *path, int die_on_error)
> {
> + static struct strbuf resolved = STRBUF_INIT;
This being 'static' would probably mean that this is not reentrant,
which goes against the title of the patch.
^ permalink raw reply
* Merging .gitmodules files
From: David Turner @ 2016-12-06 23:48 UTC (permalink / raw)
To: git@vger.kernel.org
Consider two commits: one adds file A, and the other adds file B. These commits don't conflict; you can merge them with no problem.
But if the two commits instead add submodules A and B, and you try to merge, you'll likely get a conflict in .gitmodules. This seems wrong; .gitmodules happens to be a plain text file as an implementation detail, but in terms of interpretation, it is more like a map of maps (name1 -> {path -> "...", url -> "..."}, name2 -> ...).
We (Two Sigma) keep our .gitmodules file in alphabetical order (so we don't use git submodule add -- our .gitmodules file is instead generated by some more complicated offline process). But even for ordinary .gitmodules files, order is not important so long as it's consistent.
I could set my .gitattributes for the .gitmodules file to use a custom merge driver. But: (a) I don't see an off-the-shelf one that does what I want ("union" happens to work in the add/add case, but not in the add/remove case or other cases) and (b) I would have to rewrite my whole history in order to have the .gitmodules file exist at every commit (or find a way to get .git/info/attributes into each of my users' clones) and (c) this should work correctly without customization; Git already treats the .gitmodules file as special for commands like "status"; there's no reason it shouldn't do so for merge and rebase.
I'm not sure I'll necessarily have time to implement this -- for our use case ( http://github.com/twosigma/git-meta ), we might be able to get away with doing it in JS, and using something like https://github.com/mirkokiefer/diff-merge-patch#sets . But if I did have time, do others agree that it would be reasonable to special-case this file? (Naturally, before doing the merge, we would check that the file was in fact parseable as a git config file; merging two changed gitmodules files of which either is unparseable would fall back to merging as text).
^ permalink raw reply
* What's cooking in git.git (Dec 2016, #01; Tue, 6)
From: Junio C Hamano @ 2016-12-06 23:46 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
The 'next' branch has been rewound, 'maint' now is for maintenance
fixes post v2.11 release.
The description of many new topics in this issue of the report is
probably sketchier than it should be. Refinements are very much
appreciated.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[Graduated to "master"]
* jk/common-main (2016-11-29) 1 commit
(merged to 'next' on 2016-11-29 at 2985e7efba)
+ common-main: stop munging argv[0] path
Fix for a small regression in a topic already in 'master'.
--------------------------------------------------
[New Topics]
* ak/lazy-prereq-mktemp (2016-11-29) 1 commit
- t7610: clean up foo.XXXXXX tmpdir
Test code clean-up.
Will merge to 'next'.
* da/mergetool-trust-exit-code (2016-11-29) 2 commits
- mergetools/vimdiff: trust Vim's exit code
- mergetool: honor mergetool.$tool.trustExitCode for built-in tools
mergetool.<tool>.trustExitCode configuration variable did not apply
to built-in tools, but now it does.
Will merge to 'next'.
* jb/diff-no-index-no-abbrev (2016-11-29) 1 commit
- diff: handle --no-abbrev outside of repository
"git diff --no-index" did not take "--no-abbrev" option.
Will merge to 'next'.
* vk/p4-submit-shelve (2016-11-29) 1 commit
- git-p4: allow submit to create shelved changelists.
(this branch is used by ld/p4-update-shelve.)
Will merge to 'next'.
* jk/http-walker-limit-redirect-2.9 (2016-12-06) 5 commits
- http: treat http-alternates like redirects
- http: make redirects more obvious
- remote-curl: rename shadowed options variable
- http: always update the base URL for redirects
- http: simplify update_url_from_redirect
(this branch is used by jk/http-walker-limit-redirect.)
Transport with dumb http can be fooled into following foreign URLs
that the end user does not intend to, especially with the server
side redirects and http-alternates mechanism, which can lead to
security issues. Tighten the redirection and make it more obvious
to the end user when it happens.
Will merge to 'next'.
* jk/http-walker-limit-redirect (2016-12-06) 2 commits
- http-walker: complain about non-404 loose object errors
- Merge branch 'ew/http-walker' into jk/http-walker-limit-redirect
(this branch uses jk/http-walker-limit-redirect-2.9.)
Update the error messages from the dumb-http client when it fails
to obtain loose objects; we used to give sensible error message
only upon 404 but we now forbid unexpected redirects that needs to
be reported with something sensible.
Will merge to 'next'.
* ah/grammos (2016-12-05) 3 commits
- clone,fetch: explain the shallow-clone option a little more clearly
- receive-pack: improve English grammar of denyCurrentBranch message
- bisect: improve English grammar of not-ancestors message
A few messages have been fixed for their grammatical errors.
Will merge to 'next'.
* ak/commit-only-allow-empty (2016-12-05) 1 commit
- commit: make --only --allow-empty work without paths
"git commit --allow-empty --only" (no pathspec) with dirty index
ought to be an acceptable way to create a new commit that does not
change any paths, but it was forbidden (perhaps because nobody
needed it).
Will merge to 'next'.
* bb/unicode-9.0 (2016-12-05) 3 commits
- unicode_width.h: update the tables to Unicode 9.0
- update_unicode.sh: strip the plane offsets from the double_width[] table
- update_unicode.sh: automatically download newer definition files
The character width table has been updated to match Unicode 9.0
Will merge to 'next'.
* ld/p4-update-shelve (2016-12-05) 1 commit
- git-p4: support updating an existing shelved changelist
(this branch uses vk/p4-submit-shelve.)
Will merge to 'next'.
* ld/p4-worktree (2016-12-05) 1 commit
- git-p4: support secondary working trees managed by "git worktree"
Iffy.
cf. <20161202224319.5385-2-luke@diamand.org>
* ls/p4-empty-file-on-lfs (2016-12-05) 1 commit
- git-p4: fix empty file processing for large file system backend GitLFS
"git p4" LFS support was broken when LFS stores an empty blob.
Will merge to 'next'.
* ls/p4-retry-thrice (2016-12-05) 1 commit
- git-p4: add config to retry p4 commands; retry 3 times by default
Will merge to 'next'.
* ls/t0021-fixup (2016-12-05) 1 commit
- t0021: minor filter process test cleanup
Will merge to 'next'.
* ls/travis-update-p4-and-lfs (2016-12-05) 1 commit
- travis-ci: update P4 to 16.2 and GitLFS to 1.5.2 in Linux build
The default Travis-CI configuration specifies newer P4 and GitLFS.
Will merge to 'next'.
* sb/t3600-cleanup (2016-12-05) 1 commit
- t3600: remove useless redirect
Code cleanup.
Will merge to 'next'.
* sb/unpack-trees-grammofix (2016-12-05) 1 commit
- unpack-trees: fix grammar for untracked files in directories
Will merge to 'next'.
* da/difftool-dir-diff-fix (2016-12-06) 1 commit
- difftool: fix dir-diff index creation when in a subdirectory
"git difftool --dir-diff" had a minor regression when started from
a subdirectory, which has been fixed.
Will merge to 'next'.
* jc/lockfile-silent-on-error (2016-12-06) 1 commit
- lockfile: LOCK_SILENT_ON_ERROR
Recent change to use "hold_lock*()" without die-on-error in the
merge machinery made an error from "git merge" harder to spot.
Introduce an opt-in "be silent because it has been verified that
this particular caller produces a sensible error message itself"
flag, and make everybody else show the error message when it
returns to the caller with -1 without dying.
* jk/stash-disable-renames-internally (2016-12-06) 1 commit
- stash: prefer plumbing over git-diff
When diff.renames configuration is on (and with Git 2.9 and later,
it is enabled by default, which made it worse), "git stash"
misbehaved if a file is removed and another file with a very
similar content is added.
Will merge to 'next'.
* jk/xdiff-drop-xdl-fast-hash (2016-12-06) 1 commit
- xdiff: drop XDL_FAST_HASH
Retire the "fast hash" that had disastrous performance issues in
some corner cases.
Will cook in 'next'.
* ls/filter-process (2016-12-06) 1 commit
- docs: warn about possible '=' in clean/smudge filter process values
Doc update.
Will merge to 'next'.
* nd/for-each-ref-ignore-case (2016-12-05) 1 commit
- tag, branch, for-each-ref: add --ignore-case for sorting and filtering
"git branch --list" and friends learned "--ignore-case" option to
optionally sort branches and tags case insensitively.
Will merge to 'next'.
* rj/git-version-gen-do-not-force-abbrev (2016-12-06) 1 commit
- GIT-VERSION-GEN: do not force abbreviation length used by 'describe'
A minor build update.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* jc/retire-compaction-heuristics (2016-11-02) 3 commits
- SQUASH???
- SQUASH???
- diff: retire the original experimental "compaction" heuristics
Waiting for a reroll.
* jc/abbrev-autoscale-config (2016-11-01) 1 commit
- config.abbrev: document the new default that auto-scales
Waiting for a reroll.
* jk/nofollow-attr-ignore (2016-11-02) 5 commits
- exclude: do not respect symlinks for in-tree .gitignore
- attr: do not respect symlinks for in-tree .gitattributes
- exclude: convert "check_index" into a flags field
- attr: convert "macro_ok" into a flags field
- add open_nofollow() helper
As we do not follow symbolic links when reading control files like
.gitignore and .gitattributes from the index, match the behaviour
and not follow symbolic links when reading them from the working
tree. This also tightens security a bit by not leaking contents of
an unrelated file in the error messages when it is pointed at by
one of these files that is a symbolic link.
Perhaps we want to cover .gitmodules too with the same mechanism?
* nd/worktree-move (2016-11-28) 11 commits
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
. copy.c: convert copy_file() to copy_dir_recursively()
. copy.c: style fix
. copy.c: convert bb_(p)error_msg to error(_errno)
. copy.c: delete unused code in copy_file()
. copy.c: import copy_file() from busybox
(this branch uses nd/worktree-list-fixup; is tangled with sb/submodule-embed-gitdir.)
"git worktree" learned move and remove subcommands.
Reported to break builds on Windows.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
While I think it would make it easier for people to experiment and
build on if the topic is merged to 'next', I am at the same time a
bit reluctant to merge an unproven new topic that introduces a new
file format, which we may end up having to support til the end of
time. It is likely that to support a "prime clone from CDN", it
would need a lot more than just "these are the heads and the pack
data is over there", so this may not be sufficient.
Will discard.
* mh/connect (2016-06-06) 10 commits
- connect: [host:port] is legacy for ssh
- connect: move ssh command line preparation to a separate function
- connect: actively reject git:// urls with a user part
- connect: change the --diag-url output to separate user and host
- connect: make parse_connect_url() return the user part of the url as a separate value
- connect: group CONNECT_DIAG_URL handling code
- connect: make parse_connect_url() return separated host and port
- connect: re-derive a host:port string from the separate host and port variables
- connect: call get_host_and_port() earlier
- connect: document why we sometimes call get_port after get_host_and_port
Rewrite Git-URL parsing routine (hopefully) without changing any
behaviour.
It has been two months without any support. We may want to discard
this.
* ec/annotate-deleted (2015-11-20) 1 commit
- annotate: skip checking working tree if a revision is provided
Usability fix for annotate-specific "<file> <rev>" syntax with deleted
files.
Has been waiting for a review for too long without seeing anything.
Will discard.
* dk/gc-more-wo-pack (2016-01-13) 4 commits
- gc: clean garbage .bitmap files from pack dir
- t5304: ensure non-garbage files are not deleted
- t5304: test .bitmap garbage files
- prepare_packed_git(): find more garbage
Follow-on to dk/gc-idx-wo-pack topic, to clean up stale
.bitmap and .keep files.
Has been waiting for a reroll for too long.
cf. <xmqq60ypbeng.fsf@gitster.mtv.corp.google.com>
Will discard.
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* jc/renormalize-merge-kill-safer-crlf (2016-12-01) 4 commits
- convert: git cherry-pick -Xrenormalize did not work
- Merge branch 'tb/t0027-raciness-fix' into jc/renormalize-merge-kill-safer-crlf
- merge-recursive: handle NULL in add_cacheinfo() correctly
- cherry-pick: demonstrate a segmentation fault
Fix a corner case in merge-recursive regression that crept in
during 2.10 development cycle.
Will merge to 'next'.
* js/difftool-builtin (2016-11-28) 2 commits
- difftool: implement the functionality in the builtin
- difftool: add a skeleton for the upcoming builtin
Rewrite a scripted porcelain "git difftool" in C.
Under discussion.
* nd/qsort-in-merge-recursive (2016-11-28) 1 commit
- merge-recursive.c: use string_list_sort instead of qsort
Code simplification.
Will merge to 'next'.
* bw/push-dry-run (2016-11-23) 2 commits
- push: fix --dry-run to not push submodules
- push: --dry-run updates submodules when --recurse-submodules=on-demand
(this branch uses hv/submodule-not-yet-pushed-fix; is tangled with sb/push-make-submodule-check-the-default.)
"git push --dry-run --recurse-submodule=on-demand" wasn't
"--dry-run" in the submodules.
Will merge to 'next'.
* sb/push-make-submodule-check-the-default (2016-11-29) 2 commits
- push: change submodule default to check when submodules exist
- submodule add: extend force flag to add existing repos
(this branch uses hv/submodule-not-yet-pushed-fix; is tangled with bw/push-dry-run.)
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Will merge to 'next'.
* jk/rev-parse-symbolic-parents-fix (2016-11-16) 1 commit
- rev-parse: fix parent shorthands with --symbolic
"git rev-parse --symbolic" failed with a more recent notation like
"HEAD^-1" and "HEAD^!".
Will merge to 'next'.
* nd/worktree-list-fixup (2016-11-28) 5 commits
- worktree list: keep the list sorted
- worktree.c: get_worktrees() takes a new flag argument
- get_worktrees() must return main worktree as first item even on error
- worktree: reorder an if statement
- worktree.c: zero new 'struct worktree' on allocation
(this branch is used by nd/worktree-move and sb/submodule-embed-gitdir.)
The output from "git worktree list" was made in readdir() order,
and was unstable.
Will merge to 'next'.
* jk/trailers-placeholder-in-pretty (2016-11-21) 2 commits
- ref-filter: add support to display trailers as part of contents
- pretty: add %(trailers) format for displaying trailers of a commit message
In addition to %(subject), %(body), "log --pretty=format:..."
learned a new placeholder %(trailers).
Will merge to 'next'.
* sb/submodule-embed-gitdir (2016-12-05) 5 commits
- submodule: add embed-git-dir function
- worktree: get worktrees from submodules
- test-lib-functions.sh: teach test_commit -C <dir>
- submodule helper: support super prefix
- submodule: use absolute path for computing relative path connecting
(this branch uses nd/worktree-list-fixup; is tangled with nd/worktree-move.)
A new submodule helper "git submodule embedgitdirs" to make it
easier to move embedded .git/ directory for submodules in a
superproject to .git/modules/ (and point the latter with the former
that is turned into a "gitdir:" file) has been added.
Waiting for review.
* dt/empty-submodule-in-merge (2016-11-17) 1 commit
- submodules: allow empty working-tree dirs in merge/cherry-pick
An empty directory in a working tree that can simply be nuked used
to interfere while merging or cherry-picking a change to create a
submodule directory there, which has been fixed..
Will merge to 'next'.
* bw/grep-recurse-submodules (2016-11-22) 6 commits
- grep: search history of moved submodules
- grep: enable recurse-submodules to work on <tree> objects
- grep: optionally recurse into submodules
- grep: add submodules as a grep source type
- submodules: load gitmodules file from commit sha1
- submodules: add helper functions to determine presence of submodules
"git grep" learns to optionally recurse into submodules
Has anybody else seen t7814 being flakey with this series?
* dt/smart-http-detect-server-going-away (2016-11-18) 2 commits
(merged to 'next' on 2016-12-05 at 3ea70d01af)
+ upload-pack: optionally allow fetching any sha1
+ remote-curl: don't hang when a server dies before any output
Originally merged to 'next' on 2016-11-21
When the http server gives an incomplete response to a smart-http
rpc call, it could lead to client waiting for a full response that
will never come. Teach the client side to notice this condition
and abort the transfer.
An improvement counterproposal has failed.
cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>
Will cook in 'next'.
* mm/push-social-engineering-attack-doc (2016-11-14) 1 commit
(merged to 'next' on 2016-12-05 at 9a2b5bd1a9)
+ doc: mention transfer data leaks in more places
Originally merged to 'next' on 2016-11-16
Doc update on fetching and pushing.
Will cook in 'next'.
* jc/compression-config (2016-11-15) 1 commit
(merged to 'next' on 2016-12-05 at 323769ca07)
+ compression: unify pack.compression configuration parsing
Originally merged to 'next' on 2016-11-23
Compression setting for producing packfiles were spread across
three codepaths, one of which did not honor any configuration.
Unify these so that all of them honor core.compression and
pack.compression variables the same way.
Will cook in 'next'.
* mm/gc-safety-doc (2016-11-16) 1 commit
(merged to 'next' on 2016-12-05 at 031ecc1886)
+ git-gc.txt: expand discussion of races with other processes
Originally merged to 'next' on 2016-11-17
Doc update.
Will cook in 'next'.
* hv/submodule-not-yet-pushed-fix (2016-11-16) 4 commits
(merged to 'next' on 2016-12-05 at c9d729fca2)
+ submodule_needs_pushing(): explain the behaviour when we cannot answer
+ batch check whether submodule needs pushing into one call
+ serialize collection of refs that contain submodule changes
+ serialize collection of changed submodules
(this branch is used by bw/push-dry-run and sb/push-make-submodule-check-the-default.)
Originally merged to 'next' on 2016-11-21
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
Will cook in 'next'.
* kn/ref-filter-branch-list (2016-11-15) 18 commits
- for-each-ref: do not segv with %(HEAD) on an unborn branch
- branch: implement '--format' option
- branch: use ref-filter printing APIs
- branch, tag: use porcelain output
- ref-filter: allow porcelain to translate messages in the output
- ref-filter: add `:dir` and `:base` options for ref printing atoms
- ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
- ref-filter: introduce symref_atom_parser() and refname_atom_parser()
- ref-filter: introduce refname_atom_parser_internal()
- ref-filter: make "%(symref)" atom work with the ':short' modifier
- ref-filter: add support for %(upstream:track,nobracket)
- ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
- ref-filter: introduce format_ref_array_item()
- ref-filter: move get_head_description() from branch.c
- ref-filter: modify "%(objectname:short)" to take length
- ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
- ref-filter: include reference to 'used_atom' within 'atom_value'
- ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Rerolled, reviewed, looking good.
Expecting a reroll.
cf. <20161108201211.25213-1-Karthik.188@gmail.com>
cf. <CAOLa=ZQqe3vEj_428d41vd_4kfjzsm87Wam6Zm2dhXWkPdJ8Rw@mail.gmail.com>
cf. <xmqq7f84tqa7.fsf_-_@gitster.mtv.corp.google.com>
* bw/transport-protocol-policy (2016-12-05) 5 commits
- transport: add from_user parameter to is_transport_allowed
- http: create function to get curl allowed protocols
- http: always warn if libcurl version is too old
- transport: add protocol policy config option
- lib-proto-disable: variable name fix
Finer-grained control of what protocols are allowed for transports
during clone/fetch/push have been enabled via a new configuration
mechanism.
* jt/fetch-no-redundant-tag-fetch-map (2016-11-11) 1 commit
(merged to 'next' on 2016-12-05 at 432f9469a7)
+ fetch: do not redundantly calculate tag refmap
Originally merged to 'next' on 2016-11-16
Code cleanup to avoid using redundant refspecs while fetching with
the --tags option.
Will cook in 'next'.
* sb/submodule-config-cleanup (2016-11-22) 3 commits
(merged to 'next' on 2016-12-05 at 658b8764bf)
+ submodule-config: clarify parsing of null_sha1 element
+ submodule-config: rename commit_sha1 to treeish_name
+ submodule config: inline config_from_{name, path}
Originally merged to 'next' on 2016-11-23
Minor code clean-up.
Will cook in 'next'.
* jc/push-default-explicit (2016-10-31) 2 commits
(merged to 'next' on 2016-12-05 at d63f3777af)
+ push: test pushing ambiguously named branches
+ push: do not use potentially ambiguous default refspec
Originally merged to 'next' on 2016-11-01
A lazy "git push" without refspec did not internally use a fully
specified refspec to perform 'current', 'simple', or 'upstream'
push, causing unnecessary "ambiguous ref" errors.
Will cook in 'next'.
* jt/use-trailer-api-in-commands (2016-11-29) 5 commits
- sequencer: use trailer's trailer layout
- trailer: have function to describe trailer layout
- trailer: avoid unnecessary splitting on lines
- commit: make ignore_non_trailer take buf/len
- trailer: be stricter in parsing separators
Commands that operate on a log message and add lines to the trailer
blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
"commit -s", have been taught to use the logic of and share the
code with "git interpret-trailer".
Will merge to 'next'.
* nd/rebase-forget (2016-11-28) 1 commit
- rebase: add --forget to cleanup rebase, leave everything else untouched
"git rebase" learned "--forget" option, which allows a user to
remove the metadata left by an earlier "git rebase" that was
manually aborted without using "git rebase --abort".
Will merge to 'next'.
* jc/git-open-cloexec (2016-11-02) 3 commits
- sha1_file: stop opening files with O_NOATIME
- git_open_cloexec(): use fcntl(2) w/ FD_CLOEXEC fallback
- git_open(): untangle possible NOATIME and CLOEXEC interactions
The codeflow of setting NOATIME and CLOEXEC on file descriptors Git
opens has been simplified.
We may want to drop the tip one.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/reset-unmerge (2016-10-24) 1 commit
- reset: --unmerge
After "git add" is run prematurely during a conflict resolution,
"git diff" can no longer be used as a way to sanity check by
looking at the combined diff. "git reset" learned a new
"--unmerge" option to recover from this situation.
Will discard.
This may not be needed, given that update-index has a similar
option.
* jc/merge-base-fp-only (2016-10-19) 8 commits
. merge-base: fp experiment
- merge: allow to use only the fp-only merge bases
- merge-base: limit the output to bases that are on first-parent chain
- merge-base: mark bases that are on first-parent chain
- merge-base: expose get_merge_bases_many_0() a bit more
- merge-base: stop moving commits around in remove_redundant()
- sha1_name: remove ONELINE_SEEN bit
- commit: simplify fastpath of merge-base
An experiment of merge-base that ignores common ancestors that are
not on the first parent chain.
Will discard.
The whole premise feels wrong.
* tb/convert-stream-check (2016-10-27) 2 commits
- convert.c: stream and fast search for binary
- read-cache: factor out get_sha1_from_index() helper
End-of-line conversion sometimes needs to see if the current blob
in the index has NULs and CRs to base its decision. We used to
always get a full statistics over the blob, but in many cases we
can return early when we have seen "enough" (e.g. if we see a
single NUL, the blob will be handled as binary). The codepaths
have been optimized by using streaming interface.
Will discard.
Retracted.
cf. <20161102071646.GA5094@tb-raspi>
* pb/bisect (2016-10-18) 27 commits
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Waiting for review.
* st/verify-tag (2016-10-10) 7 commits
- t/t7004-tag: Add --format specifier tests
- t/t7030-verify-tag: Add --format specifier tests
- builtin/tag: add --format argument for tag -v
- builtin/verify-tag: add --format to verify-tag
- tag: add format specifier to gpg_verify_tag
- ref-filter: add function to print single ref_array_item
- gpg-interface, tag: add GPG_VERIFY_QUIET flag
"git tag" and "git verify-tag" learned to put GPG verification
status in their "--format=<placeholders>" output format.
Waiting for a reroll.
cf. <20161007210721.20437-1-santiago@nyu.edu>
* sb/attr (2016-11-11) 35 commits
- completion: clone can initialize specific submodules
- clone: add --init-submodule=<pathspec> switch
- submodule update: add `--init-default-path` switch
- pathspec: allow escaped query values
- pathspec: allow querying for attributes
- pathspec: move prefix check out of the inner loop
- pathspec: move long magic parsing out of prefix_pathspec
- Documentation: fix a typo
- attr: keep attr stack for each check
- attr: convert to new threadsafe API
- attr: make git_check_attr_counted static
- attr.c: outline the future plans by heavily commenting
- attr.c: always pass check[] to collect_some_attrs()
- attr.c: introduce empty_attr_check_elems()
- attr.c: correct ugly hack for git_all_attrs()
- attr.c: rename a local variable check
- attr.c: pass struct git_attr_check down the callchain
- attr.c: add push_stack() helper
- attr: support quoting pathname patterns in C style
- attr: expose validity check for attribute names
- attr: add counted string version of git_check_attr()
- attr: retire git_check_attrs() API
- attr: convert git_check_attrs() callers to use the new API
- attr: convert git_all_attrs() to use "struct git_attr_check"
- attr: (re)introduce git_check_attr() and struct git_attr_check
- attr: rename function and struct related to checking attributes
- attr.c: plug small leak in parse_attr_line()
- attr.c: tighten constness around "git_attr" structure
- attr.c: simplify macroexpand_one()
- attr.c: mark where #if DEBUG ends more clearly
- attr.c: complete a sentence in a comment
- attr.c: explain the lack of attr-name syntax check in parse_attr()
- attr.c: update a stale comment on "struct match_attr"
- attr.c: use strchrnul() to scan for one line
- commit.c: use strchrnul() to scan for one line
The attributes API has been updated so that it can later be
optimized using the knowledge of which attributes are queried.
Building on top of the updated API, the pathspec machinery learned
to select only paths with given attributes set.
Waiting for review.
* va/i18n-perl-scripts (2016-11-11) 16 commits
- i18n: difftool: mark warnings for translation
- i18n: send-email: mark composing message for translation
- i18n: send-email: mark string with interpolation for translation
- i18n: send-email: mark warnings and errors for translation
- i18n: send-email: mark strings for translation
- i18n: add--interactive: mark status words for translation
- i18n: add--interactive: remove %patch_modes entries
- i18n: add--interactive: mark edit_hunk_manually message for translation
- i18n: add--interactive: i18n of help_patch_cmd
- i18n: add--interactive: mark patch prompt for translation
- i18n: add--interactive: mark plural strings
- i18n: clean.c: match string with git-add--interactive.perl
- i18n: add--interactive: mark strings with interpolation for translation
- i18n: add--interactive: mark simple here-documents for translation
- i18n: add--interactive: mark strings for translation
- Git.pm: add subroutines for commenting lines
Porcelain scripts written in Perl are getting internationalized.
Waiting for review.
* jc/latin-1 (2016-09-26) 2 commits
(merged to 'next' on 2016-12-05 at fb549caa12)
+ utf8: accept "latin-1" as ISO-8859-1
+ utf8: refactor code to decide fallback encoding
Originally merged to 'next' on 2016-09-28
Some platforms no longer understand "latin-1" that is still seen in
the wild in e-mail headers; replace them with "iso-8859-1" that is
more widely known when conversion fails from/to it.
Will cook in 'next'.
* sg/fix-versioncmp-with-common-suffix (2016-09-08) 5 commits
- versioncmp: cope with common leading parts in versionsort.prereleaseSuffix
- versioncmp: pass full tagnames to swap_prereleases()
- t7004-tag: add version sort tests to show prerelease reordering issues
- t7004-tag: use test_config helper
- t7004-tag: delete unnecessary tags with test_when_finished
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
Waiting for a reroll.
cf. <20160908223727.Horde.jVOOJ278ssZ3qkyjkmyqZD-@webmail.informatik.kit.edu>
* jc/pull-rebase-ff (2016-11-29) 1 commit
- pull: fast-forward "pull --rebase=true"
"git pull --rebase", when there is no new commits on our side since
we forked from the upstream, should be able to fast-forward without
invoking "git rebase", but it didn't.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
^ permalink raw reply
* Re: [PATCH v15 19/27] bisect--helper: `bisect_state` & `bisect_head` shell function in C
From: Stephan Beyer @ 2016-12-06 23:54 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPPTnfOYsL8OB5x5n9cm2s+chMwg=xo459kVnFV9bB+5Hw@mail.gmail.com>
Hi Pranit,
On 12/06/2016 11:40 PM, Pranit Bauva wrote:
> On Tue, Nov 22, 2016 at 5:42 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
>> On 10/14/2016 04:14 PM, Pranit Bauva wrote:
>>> +static int bisect_state(struct bisect_terms *terms, const char **argv,
>>> + int argc)
>>> +{
>>> + const char *state = argv[0];
>>> +
>>> + get_terms(terms);
>>> + if (check_and_set_terms(terms, state))
>>> + return -1;
>>> +
>>> + if (!argc)
>>> + die(_("Please call `--bisect-state` with at least one argument"));
>>
>> I think this check should move to cmd_bisect__helper. There are also the
>> other argument number checks.
>
> Not really. After the whole conversion, the cmdmode will cease to
> exists while bisect_state will be called directly, thus it is
> important to check it here.
Okay, that's a point.
In that case, you should probably use "return error()" again and the
message (mentioning "--bisect-state") does not make sense when
--bisect-state ceases to exist.
>>> +
>>> + if (argc == 1 && one_of(state, terms->term_good,
>>> + terms->term_bad, "skip", NULL)) {
>>> + const char *bisected_head = xstrdup(bisect_head());
>>> + const char *hex[1];
>>
>> Maybe:
>> const char *hex;
>>
>>> + unsigned char sha1[20];
>>> +
>>> + if (get_sha1(bisected_head, sha1))
>>> + die(_("Bad rev input: %s"), bisected_head);
>>
>> And instead of...
>>
>>> + if (bisect_write(state, sha1_to_hex(sha1), terms, 0))
>>> + return -1;
>>> +
>>> + *hex = xstrdup(sha1_to_hex(sha1));
>>> + if (check_expected_revs(hex, 1))
>>> + return -1;
>>
>> ... simply:
>>
>> hex = xstrdup(sha1_to_hex(sha1));
>> if (set_state(terms, state, hex)) {
>> free(hex);
>> return -1;
>> }
>> free(hex);
>>
>> where:
>
> Yes I am planning to convert all places with hex rather than the sha1
> but not yet, maybe in an another patch series because currently a lot
> of things revolve around sha1 and changing its behaviour wouldn't
> really be a part of a porting patch series.
I was not suggesting a change of behavior, I was suggesting a simple
helper function set_state() to get rid of code duplication above and
some lines below:
>> ... And replace this:
>>
>>> + const char **hex_string = (const char **) &hex.items[i].string;
>>> + if(bisect_write(state, *hex_string, terms, 0)) {
>>> + string_list_clear(&hex, 0);
>>> + return -1;
>>> + }
>>> + if (check_expected_revs(hex_string, 1)) {
>>> + string_list_clear(&hex, 0);
>>> + return -1;
>>> + }
>>
>> by:
>>
>> const char *hex_str = hex.items[i].string;
>> if (set_state(terms, state, hex_string)) {
>> string_list_clear(&hex, 0);
>> return -1;
>> }
See?
>>> @@ -184,8 +137,8 @@ exit code \$res from '\$command' is < 0 or >= 128" >&2
>>> state="$TERM_GOOD"
>>> fi
>>>
>>> - # We have to use a subshell because "bisect_state" can exit.
>>> - ( bisect_state $state >"$GIT_DIR/BISECT_RUN" )
>>> + # We have to use a subshell because "--bisect-state" can exit.
>>> + ( git bisect--helper --bisect-state $state >"$GIT_DIR/BISECT_RUN" )
>>
>> The new comment is funny, but you don't need a subshell here any
>> longer.
>
> True, but right now I didn't want to modify that part of the source
> code to remove the comment. I will remove the comment all together
> when I port bisect_run()
For most of the patches, I was commenting on the current state, not on
the big picture.
Still I think that it is better to remove the comment and the subshell
instead of making the comment weird ("yes the builtin can exit, but why
do we need a subshell?" vs "yes the shell function calls exit, so we
need a subshell because we do not want to exit this shell script")
~Stephan
^ permalink raw reply
* Re: Merging .gitmodules files
From: Junio C Hamano @ 2016-12-06 23:54 UTC (permalink / raw)
To: David Turner; +Cc: git@vger.kernel.org
In-Reply-To: <8d266034ae5940efbec3eef372a6ce43@exmbdft7.ad.twosigma.com>
David Turner <David.Turner@twosigma.com> writes:
> I could set my .gitattributes for the .gitmodules file to use a
> custom merge driver. But: (a) I don't see an off-the-shelf one
> that does what I want ("union" happens to work in the add/add
> case, but not in the add/remove case or other cases) and (b) I
> would have to rewrite my whole history in order to have the
> .gitmodules file exist at every commit (or find a way to get
> .git/info/attributes into each of my users' clones) and (c) this
> should work correctly without customization; Git already treats
> the .gitmodules file as special for commands like "status";
> there's no reason it shouldn't do so for merge and rebase.
> ... if I did have time, do others agree that it would be
> reasonable to special-case this file? (Naturally, before doing
> the merge, we would check that the file was in fact parseable as a
> git config file; merging two changed gitmodules files of which
> either is unparseable would fall back to merging as text).
I do not see a fundamental reason why Git shouldn't know what
.gitmodules file is and how it should merge.
Our philosophy has always been "give users enough flexibility so
that they can experiment and come up with a solution that is general
enough (i.e. you can do it with custom merge driver), and then fold
it back into the core if it is an issue that is general enough and
it makes sense for the core to care about (i.e. my "why not?"
above). If you already have a custom merge driver, then you have
already cleared the first step ;-)
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Brandon Williams @ 2016-12-07 0:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, peff, jacob.keller
In-Reply-To: <xmqqtwagy65q.fsf@gitster.mtv.corp.google.com>
On 12/06, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
> > +/* removes the last path component from 'path' except if 'path' is root */
> > +static void strip_last_component(struct strbuf *path)
> > +{
> > + if (path->len > 1) {
> > + char *last_slash = find_last_dir_sep(path->buf);
> > + strbuf_setlen(path, last_slash - path->buf);
> > + }
> > +}
>
> You use find_last_dir_sep() which takes care of "Windows uses
> backslash" issue. Is this function expected to be fed something
> like "C:\My Files\foo.txt" and more importantly "C:\My Files"? Or
> is that handled by a lot higher level up in the callchain? I am
> reacting the comparison of path->len and 1 here.
This function should accept both absolute and relative paths, which
means it should probably accept "C:\My Files". I wasn't thinking about
windows 100% of the time while writing this so I'm hoping that a windows
expert will point things like this out to me :). So in reality this
check shouldn't be (path->len > 1) but rather some function is_only_root
which would check if the strbuf only contains a string which looks like
root.
>
> Also is the input expected to be normalized? Are we expected to be
> fed something like "/a//b/./c/../d/e" and react sensibly, or is that
> handled by a lot higher level up in the callchain?
Yes it should handle paths like that sensibly.
> > +/* gets the next component in 'remaining' and places it in 'next' */
> > +static void get_next_component(struct strbuf *next, struct strbuf *remaining)
> > +{
> > + char *start = NULL;
> > + char *end = NULL;
> > +
> > + strbuf_reset(next);
> > +
> > + /* look for the next component */
> > + /* Skip sequences of multiple path-separators */
> > + for (start = remaining->buf; is_dir_sep(*start); start++)
> > + /* nothing */;
>
> Style:
> ; /* nothing */
k will fix.
>
> > + /* Find end of the path component */
> > + for (end = start; *end && !is_dir_sep(*end); end++)
> > + /* nothing */;
> > +
> > + strbuf_add(next, start, end - start);
>
> OK, so this was given "///foo/bar" in "remaining" and appended
> 'foo/' to "next". I.e. deduping of slashes is handled here.
>
> POSIX cares about treating "//" at the very beginning of the path
> specially. Is that supposed to be handled here, or by a lot higher
> level up in the callchain?
What exactly does "//" mean in this context? (I'm just naive in this
area)
>
> > + /* remove the component from 'remaining' */
> > + strbuf_remove(remaining, 0, end - remaining->buf);
> > +}
> > +
> > /* We allow "recursive" symbolic links. Only within reason, though. */
> > -#define MAXDEPTH 5
> > +#define MAXSYMLINKS 5
> >
> > /*
> > * Return the real path (i.e., absolute path, with symlinks resolved
> > @@ -21,7 +51,6 @@ int is_directory(const char *path)
> > * absolute_path().) The return value is a pointer to a static
> > * buffer.
> > *
> > * The directory part of path (i.e., everything up to the last
> > * dir_sep) must denote a valid, existing directory, but the last
> > * component need not exist. If die_on_error is set, then die with an
> > @@ -33,22 +62,16 @@ int is_directory(const char *path)
> > */
> > static const char *real_path_internal(const char *path, int die_on_error)
> > {
> > + static struct strbuf resolved = STRBUF_INIT;
>
> This being 'static' would probably mean that this is not reentrant,
> which goes against the title of the patch.
Yes I mentioned in the cover letter that this was the one last thing
that needed to be changed to make it reentrant. I wanted to get this
out for review sooner since this is the biggest change (getting rid of
the chdir() calls) and dropping the static here would just require
making the callers own the memory which should hopefully be an easy
refactor. It just would have taken a bit more time to actually do that
refactor now. I'm slowly working on it while this is being reviewed and
could be ready to send out when rerolling this patch. Sorry for having
a slightly misleading patch title :)
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH v4 1/3] update-unicode.sh: automatically download newer definition files
From: Beat Bolli @ 2016-12-07 0:17 UTC (permalink / raw)
To: Junio C Hamano, Torsten Bögershausen; +Cc: git
In-Reply-To: <xmqqmvga3yod.fsf@gitster.mtv.corp.google.com>
On 05.12.16 21:31, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@web.de> writes:
>
>> On Sat, Dec 03, 2016 at 10:00:47PM +0100, Beat Bolli wrote:
>>> Checking just for the unicode data files' existence is not sufficient;
>>> we should also download them if a newer version exists on the Unicode
>>> consortium's servers. Option -N of wget does this nicely for us.
>>>
>>> Reviewed-by: Torsten Boegershausen <tboegi@web.de>
>>
>> Minor remark (Not sure if this motivates v5, may be Junio can fix it locally?)
>> s/oe/ö/
>>
>> Beside this: Thanks again (and I learned about the -N option of wget)
>
> Will fix up while queuing (only 1/3 needs it, 2/3 has it right).
>
> Also, I'll do s/update-unicode.sh/update_unicode.sh/ on the title
> and the message to match the reality. At some point we might want
> to fix the reality to match people's expectations, though.
Thanks, Junio. This was a bit sloppy of me.
I really appreciate your regard for the small things!
Cheers, Beat
^ permalink raw reply
* (no subject)
From: jbh @ 2016-12-07 1:00 UTC (permalink / raw)
To: git
In-Reply-To: <c35518dc-5278-bd89-569c-398e70aa8dc8@drbeat.li>
unsubscribe
^ permalink raw reply
* Re: [PATCH] real_path: make real_path thread-safe
From: Ramsay Jones @ 2016-12-07 1:12 UTC (permalink / raw)
To: Brandon Williams, Junio C Hamano; +Cc: git, sbeller, peff, jacob.keller
In-Reply-To: <20161207001018.GD103573@google.com>
On 07/12/16 00:10, Brandon Williams wrote:
> On 12/06, Junio C Hamano wrote:
>> POSIX cares about treating "//" at the very beginning of the path
>> specially. Is that supposed to be handled here, or by a lot higher
>> level up in the callchain?
>
> What exactly does "//" mean in this context? (I'm just naive in this
> area)
This refers to a UNC path (ie Universal Naming Convention) which
is used to refer to servers, printers and other 'network resources'.
Although this started (many moons ago) in unix, it isn't used too
much outside of windows networks! (where it is usually denoted by
\\servername\path).
You can see the relics of unix UNC paths if you look at the wording
for basename() in the POSIX standard. Note the special treatment of
the path which 'is exactly "//"', see http://pubs.opengroup.org/onlinepubs/009695399/functions/basename.html
ATB,
Ramsay Jones
^ permalink raw reply
* git add -p with new file
From: Ariel @ 2016-12-07 1:18 UTC (permalink / raw)
To: git
If you do git add -p new_file it says:
No changes.
Which is a rather confusing message. I would expect it to show me the
content of the file in patch form, in the normal way that -p works, let me
edit it, etc.
(Note: I am aware I can do -N first, but when I specifically enter the
name of a new file I feel it should figure out what I mean.)
-Ariel
^ permalink raw reply
* [PATCH 2/2] describe: add support for multiple match patterns
From: Jacob Keller @ 2016-12-07 2:32 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
In-Reply-To: <20161207023259.29355-1-jacob.e.keller@intel.com>
From: Jacob Keller <jacob.keller@gmail.com>
Convert "--match" into a string list that accumulates patterns. If any
patterns are given, then include all tags that match at least one
pattern. This allows the user to construct multiple small patterns and
compose them. If desired, a future patch could expand functionality by
adding a new option to determine the style of combining multiple
patterns.
The primary use of this feature can be seen when trying to find which
release tag a given commit made it into. Suppose that you version all
your official releases such as "v1.2", "v1.3", "v1.4", "v2.1" and so on.
Now, you also have other tags which represent -rc releases and other
such tags. If you want to find the first major release that contains
a given commit you might try
git describe --contains --match="v?.?" <commit>
This will work as long as you have only single digits. But if you start
adding multiple digits, the pattern becomes not enough to match all the
tags you wanted while excluding the ones you didn't.
By implementing multiple --match invocations, this can be avoided by
simply passing multiple match patterns to the command:
git describe --contains --match="v[0-9].[0-9]" --match="v[0-9].[0-9][0-9]" ...
The end result is the ability to easily search tag space for which
tag included a given commit, without including -rc and other tags which
aren't interesting to you.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-describe.txt | 6 +++++-
builtin/describe.c | 31 ++++++++++++++++++++++++-------
t/t6120-describe.sh | 19 +++++++++++++++++++
3 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-describe.txt b/Documentation/git-describe.txt
index e4ac448ff565..c85f2811ce28 100644
--- a/Documentation/git-describe.txt
+++ b/Documentation/git-describe.txt
@@ -83,7 +83,11 @@ OPTIONS
--match <pattern>::
Only consider tags matching the given `glob(7)` pattern,
excluding the "refs/tags/" prefix. This can be used to avoid
- leaking private tags from the repository.
+ leaking private tags from the repository, or to shrink the scope of
+ searched tags to avoid -rc tags or similar. If given multiple a list
+ of patterns will be accumulated, and tags matching any of the patterns
+ will be considered. Use `--no-match` to clear and reset the list of
+ patterns.
--always::
Show uniquely abbreviated commit object as fallback.
diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157efc..e3ceab65e273 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -28,7 +28,7 @@ static int abbrev = -1; /* unspecified */
static int max_candidates = 10;
static struct hashmap names;
static int have_util;
-static const char *pattern;
+static struct string_list patterns = STRING_LIST_INIT_NODUP;
static int always;
static const char *dirty;
@@ -129,9 +129,25 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
if (!all && !is_tag)
return 0;
- /* Accept only tags that match the pattern, if given */
- if (pattern && (!is_tag || wildmatch(pattern, path + 10, 0, NULL)))
- return 0;
+ /*
+ * If we're given patterns, accept only tags which match at least one
+ * pattern.
+ */
+ if (patterns.nr) {
+ struct string_list_item *item;
+
+ if (!is_tag)
+ return 0;
+
+ for_each_string_list_item(item, &patterns) {
+ if (!wildmatch(item->string, path + 10, 0, NULL)) {
+ break;
+ }
+
+ /* If we get here, no pattern matched. */
+ return 0;
+ }
+ }
/* Is it annotated? */
if (!peel_ref(path, peeled.hash)) {
@@ -404,7 +420,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
N_("only output exact matches"), 0),
OPT_INTEGER(0, "candidates", &max_candidates,
N_("consider <n> most recent tags (default: 10)")),
- OPT_STRING(0, "match", &pattern, N_("pattern"),
+ OPT_STRING_LIST(0, "match", &patterns, N_("pattern"),
N_("only consider tags matching <pattern>")),
OPT_BOOL(0, "always", &always,
N_("show abbreviated commit object as fallback")),
@@ -430,6 +446,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
die(_("--long is incompatible with --abbrev=0"));
if (contains) {
+ struct string_list_item *item;
struct argv_array args;
argv_array_init(&args);
@@ -440,8 +457,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
argv_array_push(&args, "--always");
if (!all) {
argv_array_push(&args, "--tags");
- if (pattern)
- argv_array_pushf(&args, "--refs=refs/tags/%s", pattern);
+ for_each_string_list_item(item, &patterns)
+ argv_array_pushf(&args, "--refs=refs/tags/%s", item->string);
}
if (argc)
argv_array_pushv(&args, argv);
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 85f269411cb3..9e5db9b87a1f 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -182,6 +182,10 @@ check_describe "test2-lightweight-*" --tags --match="test2-*"
check_describe "test2-lightweight-*" --long --tags --match="test2-*" HEAD^
+check_describe "test1-lightweight-*" --long --tags --match="test1-*" --match="test2-*" HEAD^
+
+check_describe "test2-lightweight-*" --long --tags --match="test1-*" --no-match --match="test2-*" HEAD^
+
test_expect_success 'name-rev with exact tags' '
echo A >expect &&
tag_object=$(git rev-parse refs/tags/A) &&
@@ -206,4 +210,19 @@ test_expect_success 'describe --contains with the exact tags' '
test_cmp expect actual
'
+test_expect_success 'describe --contains and --match' '
+ echo "A^0" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ test_must_fail git describe --contains --match="B" $tagged_commit &&
+ git describe --contains --match="B" --match="A" $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'describe --contains and --no-match' '
+ echo "A^0" >expect &&
+ tagged_commit=$(git rev-parse "refs/tags/A^0") &&
+ git describe --contains --match="B" --no-match $tagged_commit >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* [PATCH 1/2] name-rev: allow passing multiple patterns using --refs
From: Jacob Keller @ 2016-12-07 2:32 UTC (permalink / raw)
To: git; +Cc: Jacob Keller
From: Jacob Keller <jacob.keller@gmail.com>
Allow git name-rev to take a string list of patterns instead of only
a single pattern. All patterns are matched inclusively, meaning that
a ref only needs to match one pattern to be included.
Add tests and documentation for this change.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Documentation/git-name-rev.txt | 4 +++-
builtin/name-rev.c | 41 +++++++++++++++++++++++++-----------
t/t6007-rev-list-cherry-pick-file.sh | 30 ++++++++++++++++++++++++++
3 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index ca28fb8e2a07..7433627db12d 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -26,7 +26,9 @@ OPTIONS
--refs=<pattern>::
Only use refs whose names match a given shell pattern. The pattern
- can be one of branch name, tag name or fully qualified ref name.
+ can be one of branch name, tag name or fully qualified ref name. If
+ given multiple times, use refs whose names match any of the given shell
+ patterns. Use `--no-refs` to clear any previous ref patterns given.
--all::
List all commits reachable from all refs
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index cd89d48b65e8..000a2a700ed3 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -108,7 +108,7 @@ static const char *name_ref_abbrev(const char *refname, int shorten_unambiguous)
struct name_ref_data {
int tags_only;
int name_only;
- const char *ref_filter;
+ struct string_list ref_filters;
};
static struct tip_table {
@@ -150,16 +150,33 @@ static int name_ref(const char *path, const struct object_id *oid, int flags, vo
if (data->tags_only && !starts_with(path, "refs/tags/"))
return 0;
- if (data->ref_filter) {
- switch (subpath_matches(path, data->ref_filter)) {
- case -1: /* did not match */
- return 0;
- case 0: /* matched fully */
- break;
- default: /* matched subpath */
- can_abbreviate_output = 1;
- break;
+ if (data->ref_filters.nr) {
+ struct string_list_item *item;
+ int matched = 0;
+
+ /* See if any of the patterns match. */
+ for_each_string_list_item(item, &data->ref_filters) {
+ /*
+ * We want to check every pattern even if we already
+ * found a match, just in case one of the later
+ * patterns could abbreviate the output.
+ */
+ switch (subpath_matches(path, item->string)) {
+ case -1: /* did not match */
+ break;
+ case 0: /* matched fully */
+ matched = 1;
+ break;
+ default: /* matched subpath */
+ matched = 1;
+ can_abbreviate_output = 1;
+ break;
+ }
}
+
+ /* If none of the patterns matched, stop now */
+ if (!matched)
+ return 0;
}
add_to_tip_table(oid->hash, path, can_abbreviate_output);
@@ -306,11 +323,11 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
{
struct object_array revs = OBJECT_ARRAY_INIT;
int all = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
- struct name_ref_data data = { 0, 0, NULL };
+ struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP };
struct option opts[] = {
OPT_BOOL(0, "name-only", &data.name_only, N_("print only names (no SHA-1)")),
OPT_BOOL(0, "tags", &data.tags_only, N_("only use tags to name the commits")),
- OPT_STRING(0, "refs", &data.ref_filter, N_("pattern"),
+ OPT_STRING_LIST(0, "refs", &data.ref_filters, N_("pattern"),
N_("only use refs matching <pattern>")),
OPT_GROUP(""),
OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
diff --git a/t/t6007-rev-list-cherry-pick-file.sh b/t/t6007-rev-list-cherry-pick-file.sh
index 1408b608eb03..d072ec43b016 100755
--- a/t/t6007-rev-list-cherry-pick-file.sh
+++ b/t/t6007-rev-list-cherry-pick-file.sh
@@ -99,6 +99,36 @@ test_expect_success '--cherry-pick bar does not come up empty (II)' '
test_cmp actual.named expect
'
+test_expect_success 'name-rev multiple --refs combine inclusive' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
+cat >expect <<EOF
+<tags/F
+$(git rev-list --left-right --right-only --cherry-pick F...E -- bar)
+EOF
+
+test_expect_success 'name-rev --refs excludes non-matched patterns' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/F" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
+cat >expect <<EOF
+$(git rev-list --left-right --cherry-pick F...E -- bar)
+EOF
+
+test_expect_success 'name-rev --no-refs clears the refs list' '
+ git rev-list --left-right --cherry-pick F...E -- bar > actual &&
+ git name-rev --stdin --name-only --refs="*tags/F" --refs="*tags/E" --no-refs --refs="*tags/G" \
+ < actual > actual.named &&
+ test_cmp actual.named expect
+'
+
cat >expect <<EOF
+tags/F
=tags/D
--
2.11.0.rc2.152.g4d04e67
^ permalink raw reply related
* Re: git add -p with new file
From: Duy Nguyen @ 2016-12-07 9:22 UTC (permalink / raw)
To: Ariel; +Cc: Git Mailing List
In-Reply-To: <alpine.DEB.2.11.1612062012540.13185@cherryberry.dsgml.com>
On Wed, Dec 7, 2016 at 8:18 AM, Ariel <asgit@dsgml.com> wrote:
>
> If you do git add -p new_file it says:
>
> No changes.
>
> Which is a rather confusing message. I would expect it to show me the
> content of the file in patch form, in the normal way that -p works, let me
> edit it, etc.
We could improve it a bit, suggesting the user to do git add -N. But
is there a point of using -p on a new file? It will be one big chunk,
you can't split anything. Perhaps maybe you want to use 'e' to edit
what's added?
> (Note: I am aware I can do -N first, but when I specifically enter the name
> of a new file I feel it should figure out what I mean.)
--
Duy
^ permalink raw reply
* 1 files changed
From: joris @ 2016-12-07 9:46 UTC (permalink / raw)
To: git
When committing a change that touches a single file, git reports :
1 files changed, 1069 insertions(+), 1063 deletions(-)
Note the plural for the single file.
Regards,
Joris Wu
Software developer
Brisbane
^ permalink raw reply
* Re: [PATCH 01/17] mv: convert to using pathspec struct interface
From: Duy Nguyen @ 2016-12-07 10:07 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481061106-117775-2-git-send-email-bmwill@google.com>
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> Convert the 'internal_copy_pathspec()' function to use the pathspec
> struct interface from using the deprecated 'get_pathspec()' interface.
>
> In addition to this, fix a memory leak caused by only duplicating some
> of the pathspec elements. Instead always duplicate all of the the
> pathspec elements as an intermediate step (with modificationed based on
> the passed in flags). This way the intermediate strings can then be
> freed prior to duplicating the result of parse_pathspec (which contains
> each of the elements with the prefix prepended).
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> builtin/mv.c | 45 ++++++++++++++++++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 13 deletions(-)
Stefan did something similar last year [1] but I couldn't find why it
did not get merged. Not sure if the reasons are still relevant or
not...
[1] http://public-inbox.org/git/%3C1438885632-26470-1-git-send-email-sbeller@google.com%3E/
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 2f43877..4df4a12 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -4,6 +4,7 @@
> * Copyright (C) 2006 Johannes Schindelin
> */
> #include "builtin.h"
> +#include "pathspec.h"
> #include "lockfile.h"
> #include "dir.h"
> #include "cache-tree.h"
> @@ -25,25 +26,43 @@ static const char **internal_copy_pathspec(const char *prefix,
> {
> int i;
> const char **result;
> + struct pathspec ps;
> ALLOC_ARRAY(result, count + 1);
> - COPY_ARRAY(result, pathspec, count);
> - result[count] = NULL;
> +
> + /* Create an intermediate copy of the pathspec based on the flags */
> for (i = 0; i < count; i++) {
> - int length = strlen(result[i]);
> + int length = strlen(pathspec[i]);
> int to_copy = length;
> + char *it;
> while (!(flags & KEEP_TRAILING_SLASH) &&
> - to_copy > 0 && is_dir_sep(result[i][to_copy - 1]))
> + to_copy > 0 && is_dir_sep(pathspec[i][to_copy - 1]))
> to_copy--;
> - if (to_copy != length || flags & DUP_BASENAME) {
> - char *it = xmemdupz(result[i], to_copy);
> - if (flags & DUP_BASENAME) {
> - result[i] = xstrdup(basename(it));
> - free(it);
> - } else
> - result[i] = it;
> - }
> +
> + it = xmemdupz(pathspec[i], to_copy);
> + if (flags & DUP_BASENAME) {
> + result[i] = xstrdup(basename(it));
> + free(it);
> + } else
> + result[i] = it;
> + }
> + result[count] = NULL;
> +
> + parse_pathspec(&ps,
> + PATHSPEC_ALL_MAGIC &
> + ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
> + PATHSPEC_KEEP_ORDER | PATHSPEC_PREFER_CWD,
> + prefix, result);
> + assert(count == ps.nr);
> +
> + /* Copy the pathspec and free the old intermediate strings */
> + for (i = 0; i < count; i++) {
> + const char *match = xstrdup(ps.items[i].match);
> + free((char *) result[i]);
> + result[i] = match;
Sigh.. it looks so weird that we do all the parsing (in a _copy_
pathspec function) then remove struct pathspec and return the plain
string. I guess we can't do anything more until we rework cmd_mv code
to handle pathspec natively.
At the least I think we should rename this function to something else.
But if you have time I really wish we could kill this function. I
haven't stared at cmd_mv() long and hard, but it looks to me that we
combining two separate functionalities in the same function here.
If "mv" takes n arguments, then the first <n-1> arguments may be
pathspec, the last one is always a plain path. The "dest_path =
internal_copy_pathspec..." could be as simple as "dest_path =
prefix_path(argv[argc - 1])". the special treatment for this last
argument [1] can live here. Then, we can do parse_pathspec for the
<n-1> arguments in cmd_mv(). It's still far from perfect, because
cmd_mv can't handle pathspec properly, but it reduces the messy mess
in internal_copy_pathspec a bit, I hope.
[1] c57f628 (mv: let 'git mv file no-such-dir/' error out - 2013-12-03)
> }
> - return get_pathspec(prefix, result);
> +
> + clear_pathspec(&ps);
> + return result;
> }
>
> static const char *add_slash(const char *path)
> --
> 2.8.0.rc3.226.g39d4020
>
--
Duy
^ permalink raw reply
* [PATCH v2] difftool: fix dir-diff index creation when in a subdirectory
From: David Aguilar @ 2016-12-07 10:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git ML, Frank Becker, Johannes Schindelin, John Keeping
9ec26e7977 (difftool: fix argument handling in subdirs, 2016-07-18)
corrected how path arguments are handled in a subdirectory, but
it introduced a regression in how entries outside of the
subdirectory are handled by dir-diff.
When preparing the right-side of the diff we only include the
changed paths in the temporary area.
The left side of the diff is constructed from a temporary
index that is built from the same set of changed files, but it
was being constructed from within the subdirectory. This is a
problem because the indexed paths are toplevel-relative, and
thus they were not getting added to the index.
Teach difftool to chdir to the toplevel of the repository before
preparing its temporary indexes. This ensures that all of the
toplevel-relative paths are valid.
Add test cases to more thoroughly exercise this scenario.
Reported-by: Frank Becker <fb@mooflu.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
Changes since v1:
- Improved commit message thanks to Johannes Schindelin
("git whatis" was used to describe the referenced commit)
- Add more tests for edge cases that are properly handled
but were not being tested.
git-difftool.perl | 4 ++++
t/t7800-difftool.sh | 44 +++++++++++++++++++++++++++++++++++++++++---
2 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/git-difftool.perl b/git-difftool.perl
index a5790d03a0..959822d5f3 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -182,6 +182,10 @@ EOF
}
}
+ # Go to the root of the worktree so that the left index files
+ # are properly setup -- the index is toplevel-relative.
+ chdir($workdir);
+
# Setup temp directories
my $tmpdir = tempdir('git-difftool.XXXXX', CLEANUP => 0, TMPDIR => 1);
my $ldir = "$tmpdir/left";
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 70a2de461a..99d4123461 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -374,6 +374,7 @@ test_expect_success PERL 'setup change in subdirectory' '
echo master >sub/sub &&
git add sub/sub &&
git commit -m "added sub/sub" &&
+ git tag v1 &&
echo test >>file &&
echo test >>sub/sub &&
git add file sub/sub &&
@@ -409,12 +410,49 @@ run_dir_diff_test 'difftool --dir-diff ignores --prompt' '
grep file output
'
-run_dir_diff_test 'difftool --dir-diff from subdirectory' '
+run_dir_diff_test 'difftool --dir-diff branch from subdirectory' '
(
cd sub &&
git difftool --dir-diff $symlinks --extcmd ls branch >output &&
- grep sub output &&
- grep file output
+ # "sub" must only exist in "right"
+ # "file" and "file2" must be listed in both "left" and "right"
+ test "1" = $(grep sub output | wc -l) &&
+ test "2" = $(grep file"$" output | wc -l) &&
+ test "2" = $(grep file2 output | wc -l)
+ )
+'
+
+run_dir_diff_test 'difftool --dir-diff v1 from subdirectory' '
+ (
+ cd sub &&
+ git difftool --dir-diff $symlinks --extcmd ls v1 >output &&
+ # "sub" and "file" exist in both v1 and HEAD.
+ # "file2" is unchanged.
+ test "2" = $(grep sub output | wc -l) &&
+ test "2" = $(grep file output | wc -l) &&
+ test "0" = $(grep file2 output | wc -l)
+ )
+'
+
+run_dir_diff_test 'difftool --dir-diff branch from subdirectory w/ pathspec' '
+ (
+ cd sub &&
+ git difftool --dir-diff $symlinks --extcmd ls branch -- .>output &&
+ # "sub" only exists in "right"
+ # "file" and "file2" must not be listed
+ test "1" = $(grep sub output | wc -l) &&
+ test "0" = $(grep file output | wc -l)
+ )
+'
+
+run_dir_diff_test 'difftool --dir-diff v1 from subdirectory w/ pathspec' '
+ (
+ cd sub &&
+ git difftool --dir-diff $symlinks --extcmd ls v1 -- .>output &&
+ # "sub" exists in v1 and HEAD
+ # "file" is filtered out by the pathspec
+ test "2" = $(grep sub output | wc -l) &&
+ test "0" = $(grep file output | wc -l)
)
'
--
2.11.0.1.g7697df0
^ permalink raw reply related
* Re: [PATCH 02/17] dir: convert create_simplify to use the pathspec struct interface
From: Duy Nguyen @ 2016-12-07 10:17 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481061106-117775-3-git-send-email-bmwill@google.com>
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> Convert 'create_simplify()' to use the pathspec struct interface from
> using the '_raw' entry in the pathspec.
It would be even better to kill this create_simplify() and let
simplify_away() handle struct pathspec directly.
There is a bug in this code, that might have been found if we
simpify_away() handled pathspec directly: the memcmp() in
simplify_away() will not play well with :(icase) magic. My bad. If
:(icase) is used, the easiest/safe way is simplify nothing. Later on
maybe we can teach simplify_away() to do strncasecmp instead. We could
ignore exclude patterns there too (although not excluding is not a
bug).
--
Duy
^ permalink raw reply
* Re: [PATCH 03/17] dir: convert fill_directory to use the pathspec struct interface
From: Duy Nguyen @ 2016-12-07 10:24 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481061106-117775-4-git-send-email-bmwill@google.com>
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> Convert 'fill_directory()' to use the pathspec struct interface from
> using the '_raw' entry in the pathspec struct.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> dir.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/dir.c b/dir.c
> index 7df292b..8730a4f 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -188,7 +188,8 @@ int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec)
> len = common_prefix_len(pathspec);
>
> /* Read the directory and prune it */
> - read_directory(dir, pathspec->nr ? pathspec->_raw[0] : "", len, pathspec);
> + read_directory(dir, pathspec->nr ? pathspec->items[0].match : "",
> + len, pathspec);
Or even better, use common_prefix()'s return value here. I took me a
while to realize this code was not buggy. It is fine to just pick the
first item because the first <len> characters of _all_ pathspec items
must be the same. Something like this
prefix = common_prefix(..)
read_directory(..., prefix, strlen(prefix), pathspec);
expresses it much better. Yeah one extra mem allocation, no big deal
since fill_directory() is not called very often.
> return len;
> }
>
> --
> 2.8.0.rc3.226.g39d4020
>
--
Duy
^ permalink raw reply
* Re: [PATCH 04/17] ls-tree: convert show_recursive to use the pathspec struct interface
From: Duy Nguyen @ 2016-12-07 10:38 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481061106-117775-5-git-send-email-bmwill@google.com>
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> Convert 'show_recursive()' to use the pathspec struct interface from
> using the '_raw' entry in the pathspec struct.
Slightly off-topic (sorry, but you made me look at this code! :D),
could you update the magic_mask argument of parse_pathspec() in this
file to PATHSPEC_ALL_MAGIC & ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL)?
It makes sure all future magic will be caught as unsupported (and I
think Stefan is adding one, but understandably he did not find this
code).
I think it's in the spirit of renaming _raw to match too. By limiting
magic to fromtop and literal, we are sure match can only be path and
nothing else, which is good because this show_recursive can't handle
anything else either.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> builtin/ls-tree.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
> index 0e30d86..e0f4307 100644
> --- a/builtin/ls-tree.c
> +++ b/builtin/ls-tree.c
> @@ -31,21 +31,18 @@ static const char * const ls_tree_usage[] = {
>
> static int show_recursive(const char *base, int baselen, const char *pathname)
> {
> - const char **s;
> + int i;
>
> if (ls_options & LS_RECURSIVE)
> return 1;
>
> - s = pathspec._raw;
> - if (!s)
> + if (!pathspec.nr)
> return 0;
>
> - for (;;) {
> - const char *spec = *s++;
> + for (i = 0; i < pathspec.nr; i++) {
> + const char *spec = pathspec.items[i].match;
> int len, speclen;
>
> - if (!spec)
> - return 0;
> if (strncmp(base, spec, baselen))
> continue;
> len = strlen(pathname);
> @@ -59,6 +56,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
> continue;
> return 1;
> }
> + return 0;
> }
>
> static int show_tree(const unsigned char *sha1, struct strbuf *base,
> --
> 2.8.0.rc3.226.g39d4020
>
--
Duy
^ permalink raw reply
* Re: [PATCH 09/17] pathspec: always show mnemonic and name in unsupported_magic
From: Duy Nguyen @ 2016-12-07 10:44 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481061106-117775-10-git-send-email-bmwill@google.com>
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> @@ -413,10 +411,9 @@ void parse_pathspec(struct pathspec *pathspec,
> prefixlen = prefix ? strlen(prefix) : 0;
>
> for (i = 0; i < n; i++) {
> - unsigned short_magic;
> entry = argv[i];
>
> - item[i].magic = prefix_pathspec(item + i, &short_magic,
> + item[i].magic = prefix_pathspec(item + i,
> flags,
> prefix, prefixlen, entry);
The final output looks a bit ...um.. strangely tall, with the first
two lines that have one argument each, then the last line comes with
three arguments. Maybe put 'flags' in the same line as 'item + i'?
> if ((flags & PATHSPEC_LITERAL_PATH) &&
>
--
Duy
^ permalink raw reply
* Re: [PATCH 09/17] pathspec: always show mnemonic and name in unsupported_magic
From: Duy Nguyen @ 2016-12-07 10:47 UTC (permalink / raw)
To: Brandon Williams; +Cc: Git Mailing List, Stefan Beller, Junio C Hamano
In-Reply-To: <1481061106-117775-10-git-send-email-bmwill@google.com>
On Wed, Dec 7, 2016 at 4:51 AM, Brandon Williams <bmwill@google.com> wrote:
> @@ -426,8 +423,7 @@ void parse_pathspec(struct pathspec *pathspec,
> nr_exclude++;
> if (item[i].magic & magic_mask)
> unsupported_magic(entry,
> - item[i].magic & magic_mask,
> - short_magic);
> + item[i].magic & magic_mask);
Same here. Maybe put both arguments in the same line. It looks a bit
better. (sorry for two mails on the same patch, I'm reading the final
output first before going through individual patches that breaks this
function down)
>
> if ((flags & PATHSPEC_SYMLINK_LEADING_PATH) &&
> has_symlink_leading_path(item[i].match, item[i].len)) {
--
Duy
^ permalink raw reply
* [ANNOUNCE] DeepGit - free cross-platform Git blame GUI with code move detection
From: Marc Strapetz @ 2016-12-07 10:26 UTC (permalink / raw)
To: git
We have just released the first version of DeepGit, a free
cross-platform tool to investigate the history of source code. It's
based on blame information and comes with a 'fuzzy' line/block move
detection.
http://www.syntevo.com/deepgit/
-Marc
^ permalink raw reply
* Re: [PATCH] git-p4: add p4 shelf support
From: Luke Diamand @ 2016-12-07 11:09 UTC (permalink / raw)
To: Nuno Subtil; +Cc: Git Users, Junio C Hamano, Vinicius Kursancew, Lars Schneider
In-Reply-To: <CALdXDfdXtc+FpePnGmE6_YDQ+8=wUZRqh4Xb65NJXUXaZnmELA@mail.gmail.com>
On 6 December 2016 at 16:12, Nuno Subtil <subtil@gmail.com> wrote:
> Yeah, it looks similar. This change can be ignored if those have already
> been accepted.
Thanks, that's appreciated!
Luke
^ permalink raw reply
* Re: 1 files changed
From: Duy Nguyen @ 2016-12-07 11:16 UTC (permalink / raw)
To: joris; +Cc: Git Mailing List
In-Reply-To: <5cfee62d-0b47-897d-0882-387de68d01fa@gmail.com>
On Wed, Dec 7, 2016 at 4:46 PM, joris <joriswu@gmail.com> wrote:
> When committing a change that touches a single file, git reports :
>
> 1 files changed, 1069 insertions(+), 1063 deletions(-)
>
> Note the plural for the single file.
What version? This should be fixed since v1.7.10, unless some new bug
pops up somewhere.
--
Duy
^ permalink raw reply
* Re: [PATCH v15 12/27] bisect--helper: `get_terms` & `bisect_terms` shell function in C
From: Pranit Bauva @ 2016-12-07 12:06 UTC (permalink / raw)
To: Stephan Beyer; +Cc: Git List
In-Reply-To: <a492fb63-3642-1f97-4668-a927352169f9@gmx.net>
Hey Stephan,
On Wed, Dec 7, 2016 at 4:35 AM, Stephan Beyer <s-beyer@gmx.net> wrote:
> Hey Pranit,
>
> On 12/06/2016 10:14 PM, Pranit Bauva wrote:
>>>> +
>>>> + if (argc == 0) {
>>>> + printf(_("Your current terms are %s for the old state\nand "
>>>> + "%s for the new state.\n"), terms->term_good,
>>>> + terms->term_bad);
>>>
>>> Very minor: It improves the readability if you'd split the string after
>>> the \n and put the "and "in the next line.
>>
>> Ah. This is because of the message. If I do the other way, then it
>> won't match the output in one of the tests in t/t6030 thus, I am
>> keeping it that way in order to avoid modifying the file t/t6030.
>
> I think I was unclear here. I was referring to the coding/layouting
> style, not to the string. I mean like writing:
>
> printf(_("Your current terms are %s for the old state\n"
> "and "%s for the new state.\n"),
> terms->term_good, terms->term_bad);
>
> The string fed to _() is the same, but it is split in a different (imho
> more readable) way: after the "\n", not after the "and ".
Thanks for clearing it out. This seems a sensible change.
>>>> + die(_("invalid argument %s for 'git bisect "
>>>> + "terms'.\nSupported options are: "
>>>> + "--term-good|--term-old and "
>>>> + "--term-bad|--term-new."), argv[i]);
>>>
>>> Hm, "return error(...)" and "die(...)" seems to be quasi-equivalent in
>>> this case. Because I am always looking from a library perspective, I'd
>>> prefer "return error(...)".
>>
>> I should use return error()
>
> When you reroll your patches, please also check if you always put _()
> around your error()s ;) (Hmmm... On the other hand, it might be arguable
> if translations are useful for errors that only occur when people hack
> git-bisect or use the bisect--helper directly... This makes me feel like
> all those errors should be prefixed by some "BUG: " marker since the
> ordinary user only sees them when there is a bug. But I don't feel in
> the position to decide or recommend such a thing, so it's just a thought.)
It is seems a good change, I will do it. Let other's comment on what
they think in the next re-roll.
Regards,
Pranit Bauva
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox