summaryrefslogtreecommitdiff
path: root/t/t1506-rev-parse-diagnosis.sh
AgeCommit message (Collapse)AuthorFilesLines
2020-11-10rev-parse: handle --end-of-optionsLibravatar Jeff King1-0/+16
We taught rev-list a new way to separate options from revisions in 19e8789b23 (revision: allow --end-of-options to end option parsing, 2019-08-06), but rev-parse uses its own parser. It should know about --end-of-options not only for consistency, but because it may be presented with similarly ambiguous cases. E.g., if a caller does: git rev-parse "$rev" -- "$path" to parse an untrusted input, then it will get confused if $rev contains an option-like string like "--local-env-vars". Or even "--not-real", which we'd keep as an option to pass along to rev-list. Or even more importantly: git rev-parse --verify "$rev" can be confused by options, even though its purpose is safely parsing untrusted input. On the plus side, it will always fail the --verify part, as it will not have parsed a revision, so the caller will generally "fail closed" rather than continue to use the untrusted string. But it will still trigger whatever option was in "$rev"; this should be mostly harmless, since rev-parse options are all read-only, but I didn't carefully audit all paths. This patch lets callers write: git rev-parse --end-of-options "$rev" -- "$path" and: git rev-parse --verify --end-of-options "$rev" which will both treat "$rev" always as a revision parameter. The latter is a bit clunky. It would be nicer if we had defined "--verify" to require that its next argument be the revision. But we have not historically done so, and: git rev-parse --verify -q "$rev" does currently work. I added a test here to confirm that we didn't break that. A few implementation notes: - We don't document --end-of-options explicitly in commands, but rather in gitcli(7). So I didn't give it its own section in git-rev-parse(1). But I did call it out specifically in the --verify section, and include it in the examples, which should show best practices. - We don't have to re-indent the main option-parsing block, because we can combine our "did we see end of options" check with "does it start with a dash". The exception is the pre-setup options, which need their own block. - We do however have to pull the "--" parsing out of the "does it start with dash" block, because we want to parse it even if we've seen --end-of-options. - We'll leave "--end-of-options" in the output. This is probably not technically necessary, as a careful caller will do: git rev-parse --end-of-options $revs -- $paths and anything in $revs will be resolved to an object id. However, it does help a slightly less careful caller like: git rev-parse --end-of-options $revs_or_paths where a path "--foo" will remain in the output as long as it also exists on disk. In that case, it's helpful to retain --end-of-options to get passed along to rev-list, s it would otherwise see just "--foo". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-10rev-parse: don't accept options after dashdashLibravatar Jeff King1-0/+9
Because of the order in which we check options in rev-parse, there are a few options we accept even after a "--". This is wrong, because the whole point of "--" is to say "everything after here is a path". Let's move the "did we see a dashdash" check (it's called "as_is" in the code) to the top of the parsing loop. Note there is one subtlety here. The options are ordered so that some are checked before we even see if we're in a repository (they continue the loop, and if we get past a certain point, then we do the repository setup). By moving the as_is check higher, it's also in that "before setup" section, even though it might look at the repository via verify_filename(). However, this works out: we'd never set as_is until we parse "--", and we don't parse that until after doing the setup. An alternative here to avoid the subtlety is to put the as_is check at the top of the post-setup options. But then every pre-setup option would have to remember to check "if (!as_is && !strcmp(...))". So while this is a bit magical, it's harder for future code to get wrong. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-25t1506: rev-parse A..B and A...BLibravatar Junio C Hamano1-0/+18
Because these constructs can be used to parse user input to be passed to rev-list --objects, e.g. range=$(git rev-parse v1.0..v2.0) && git rev-list --objects $range | git pack-objects --stdin the endpoints (v1.0 and v2.0 in the example) are shown without peeling them to underlying commits, even when they are annotated tags. Make sure it stays that way. While at it, ensure "rev-parse A...B" also keeps the endpoints A and B unpeeled, even though the negative side (i.e. the merge-base between A and B) has to become a commit. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-20t: use update-ref and show-ref to reading/writing refsLibravatar Han-Wen Nienhuys1-1/+1
Reading and writing .git/refs/* assumes that refs are stored in the 'files' ref backend. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-14Merge branch 'jk/get-oid-error-message-i18n'Libravatar Junio C Hamano1-24/+24
A low-level API function get_oid(), that accepts various ways to name an object, used to issue end-user facing error messages without l10n, which has been updated to be translatable. * jk/get-oid-error-message-i18n: sha1-name: mark get_oid() error messages for translation t1506: drop space after redirection operator t1400: avoid "test" string comparisons
2020-02-12Merge branch 'jk/escaped-wildcard-dwim'Libravatar Junio C Hamano1-0/+14
Disambiguation logic to tell revisions and pathspec apart has been tweaked so that backslash-escaped glob special characters do not count in the "wildcards are pathspec" rule. * jk/escaped-wildcard-dwim: verify_filename(): handle backslashes in "wildcards are pathspecs" rule
2020-01-30sha1-name: mark get_oid() error messages for translationLibravatar Jeff King1-13/+13
There are several error messages in get_oid() and its children that are clearly intended for humans, but aren't marked for translation. E.g.: $ git show :1:foo fatal: Path 'foo' is in the index, but not at stage 1. Did you mean ':0:foo'? Let's mark these for translation. While we're at it, let's switch the style to be more like our usual error messages: start with a lowercase letter and omit a period at the end of the line. This does mean that multi-line messages like the one above don't have any punctuation between the two sentences. I solved that by adding a "hint" marker like we'd see from advise(). So the result is: $ git show :1:foo fatal: path 'foo' is in the index, but not at stage 1 hint: Did you mean ':0:foo'? A few tests had to be switched to test_i18ngrep and test_i18ncmp. Since we were touching them anyway, I also simplified the ones using i18ngrep a bit for readability. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-28t1506: drop space after redirection operatorLibravatar Jeff King1-11/+11
Some (but not all!) redirections in this file are spelled "2> error". Let's switch them to our usual style. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-27verify_filename(): handle backslashes in "wildcards are pathspecs" ruleLibravatar Jeff King1-0/+14
Commit 28fcc0b71a (pathspec: avoid the need of "--" when wildcard is used, 2015-05-02) allowed: git rev-parse '*.c' without the double-dash. But the rule it uses to check for wildcards actually looks for any glob special. This is overly liberal, as it means that a pattern that doesn't actually do any wildcard matching, like "a\b", will be considered a pathspec. If you do have such a file on disk, that's presumably what you wanted. But if you don't, the results are confusing: rather than say "there's no such path a\b", we'll quietly accept it as a pathspec which very likely matches nothing (or at least not what you intended). Likewise, looking for path "a\*b" doesn't expand the search at all; it would only find a single entry, "a*b". This commit switches the rule to trigger only when glob metacharacters would expand the search, meaning both of those cases will now report an error (you can still disambiguate using "--", of course; we're just tightening the DWIM heuristic). Note that we didn't test the original feature in 28fcc0b71a at all. So this patch not only tests for these corner cases, but also adds a regression test for the existing behavior. Reported-by: David Burström <davidburstrom@spotify.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-27test: use test_must_be_empty F instead of test -z $(cat F)Libravatar René Scharfe1-4/+4
Use test_must_be_empty instead of reading the file and comparing its contents to an empty string. That's more efficient, as the function only needs built-in meta-data only check in the usual case, and provides nicer debug output otherwise. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07Merge branch 'rs/nth-parent-parse'Libravatar Junio C Hamano1-0/+8
The object name parser for "Nth parent" syntax has been made more robust against integer overflows. * rs/nth-parent-parse: sha1-name: check for overflow of N in "foo^N" and "foo~N" rev-parse: demonstrate overflow of N for "foo^N" and "foo~N"
2019-09-16sha1-name: check for overflow of N in "foo^N" and "foo~N"Libravatar René Scharfe1-2/+2
Reject values that don't fit into an int, as get_parent() and get_nth_ancestor() cannot handle them. That's better than potentially returning a random object. If this restriction turns out to be too tight then we can switch to a wider data type, but we'd still have to check for overflow. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-16rev-parse: demonstrate overflow of N for "foo^N" and "foo~N"Libravatar René Scharfe1-0/+8
If the number gets too high for an int, weird things may happen, as signed overflows are undefined. Add a test to show this; rev-parse "sucessfully" interprets 100000000000000000000000000000000 to be the same as 0, at least on x64 with GCC 9.2.1 and Clang 8.0.1, which is obviously bogus. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-06t: use common $SQ variableLibravatar Denton Liu1-3/+2
In many test scripts, there are bespoke definitions of the single quote that are some variation of this: SQ="'" Define a common $SQ variable in test-lib.sh and replace all usages of these bespoke variables with the common one. This change was done by running `git grep =\"\'\" t/` and `git grep =\\\\\'` and manually changing the resulting definitions and corresponding usages. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-13Mark messages for translationsLibravatar Alexander Shopov1-1/+1
Small changes in messages to fit the style and typography of rest. Reuse already translated messages if possible. Do not translate messages aimed at developers of git. Fix unit tests depending on the original string. Use `test_i18ngrep` for tests with translatable strings. Change and verify rest of tests via `make GETTEXT_POISON=1 test`. Signed-off-by: Alexander Shopov <ash@kambanaria.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17i18n: setup: mark strings for translationLibravatar Vasco Almeida1-1/+1
Update tests that compare the strings newly marked for translation to succeed when running under GETTEXT_POISON. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-06setup: make startup_info available everywhereLibravatar Jeff King1-5/+0
Commit a60645f (setup: remember whether repository was found, 2010-08-05) introduced the startup_info structure, which records some parts of the setup_git_directory() process (notably, whether we actually found a repository or not). One of the uses of this data is for functions to behave appropriately based on whether we are in a repo. But the startup_info struct is just a pointer to storage provided by the main program, and the only program that sets it up is the git.c wrapper. Thus builtins have access to startup_info, but externally linked programs do not. Worse, library code which is accessible from both has to be careful about accessing startup_info. This can be used to trigger a die("BUG") via get_sha1(): $ git fast-import <<-\EOF tag foo from HEAD:./whatever EOF fatal: BUG: startup_info struct is not initialized. Obviously that's fairly nonsensical input to feed to fast-import, but we should never hit a die("BUG"). And there may be other ways to trigger it if other non-builtins resolve sha1s. So let's point the storage for startup_info to a static variable in setup.c, making it available to all users of the library code. We _could_ turn startup_info into a regular extern struct, but doing so would mean tweaking all of the existing use sites. So let's leave the pointer indirection in place. We can, however, drop any checks for NULL, as they will always be false (and likewise, we can drop the test covering this case, which was a rather artificial situation using one of the test-* programs). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-12-09rev-parse: correctly diagnose revision errors before "--"Libravatar Jeff King1-0/+24
Rev-parse understands that a "--" may separate revisions and filenames, and that anything after the "--" is taken as-is. However, it does not understand that anything before the token must be a revision (which is the usual rule implemented by the setup_revisions parser). Since rev-parse prefers revisions to files when parsing before the "--", we end up with the correct result (if such an argument is a revision, we parse it as one, and if it is not, it is an error either way). However, we misdiagnose the errors: $ git rev-parse foobar -- >/dev/null fatal: ambiguous argument 'foobar': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' $ >foobar $ git rev-parse foobar -- >/dev/null fatal: bad flag '--' used after filename In both cases, we should know that the real error is that "foobar" is meant to be a revision, but could not be resolved. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-23specifying ranges: we did not mean to make ".." an empty setLibravatar Junio C Hamano1-0/+14
Either end of revision range operator can be omitted to default to HEAD, as in "origin.." (what did I do since I forked) or "..origin" (what did they do since I forked). But the current parser interprets ".." as an empty range "HEAD..HEAD", and worse yet, because ".." does exist on the filesystem, we get this annoying output: $ cd Documentation/howto $ git log .. ;# give me recent commits that touch Documentation/ area. fatal: ambiguous argument '..': both revision and filename Use '--' to separate filenames from revisions Surely we could say "git log ../" or even "git log -- .." to disambiguate, but we shouldn't have to. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-18verify_filename(): ask the caller to chose the kind of diagnosisLibravatar Matthieu Moy1-1/+1
verify_filename() can be called in two different contexts. Either we just tried to interpret a string as an object name, and it fails, so we try looking for a working tree file (i.e. we finished looking at revs that come earlier on the command line, and the next argument must be a pathname), or we _know_ that we are looking for a pathname, and shouldn't even try interpreting the string as an object name. For example, with this change, we get: $ git log COPYING HEAD:inexistant fatal: HEAD:inexistant: no such path in the working tree. Use '-- <path>...' to specify paths that do not exist locally. $ git log HEAD:inexistant fatal: Path 'inexistant' does not exist in 'HEAD' Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-06-18sha1_name: do not trigger detailed diagnosis for file argumentsLibravatar Matthieu Moy1-0/+11
diagnose_invalid_sha1_path() is meant to be called to diagnose a misspelt <treeish>:<pathname> when <pathname> does not exist in <treeish>. However, the code may call it if <treeish>:<pathname> is invalid (which triggers another call with only_to_die == 1), but for another reason. This happens when calling e.g. git log existing-file HEAD:existing-file because existing-file is a path and not a revision, the code verifies that the arguments that follow to be paths. This leads to an incorrect message like "existing-file does not exist in HEAD", even though the path exists in HEAD. Check that the search for <pathname> in <treeish> fails before triggering the diagnosis. Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-05-09t1507: avoid "${parameter<op>'word'}" inside double-quotesLibravatar Junio C Hamano1-2/+5
Kacper Kornet noticed that a $variable in "word" in the above construct is not substituted by his pdksh. Modern POSIX compliant shells (e.g. dash, ksh, bash) all seem to interpret POSIX "2.6.2 Parameter Expansion" that says "word shall be subjected to tilde expansion, parameter expansion, command substitution, and arithmetic expansion" in ${parameter<op>word}, to mean that the word is expanded as if it appeared in dq pairs, so if the word were "'$variable'" (sans dq) it would expand to a single quote, the value of the $variable and then a single quote. Johannes Sixt reports that the behavior of quoting at the right of :- when the ${...:-...} expansion appears in double-quotes was debated recently at length at the Austin group. We can avoid this issue and future-proof the test by a slight rewrite. Helped-by: Johannes Sixt <j.sixt@viscovery.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-04-03sha1_name: Suggest commit:./file for path in subdirLibravatar Michael J Gruber1-1/+1
Currently, the "Did you mean..." message suggests "commit:fullpath" only. Extend this to show the more convenient "commit:./file" form also. Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-04-03t1506: factor out test for "Did you mean..."Libravatar Michael J Gruber1-5/+12
With the current code, it's a "'"'"'" jungle, and we test only 1 line of the 2 line response. Factor out and test both. Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-12-09get_sha1: teach ":$n:<path>" the same relative path logicLibravatar Junio C Hamano1-0/+24
We taught the object name parser to take ":./<path>", ":../<path>", etc. and understand them to be relative to the current working directory. Given that ":<path>" is just a short-hand for ":0:<path>" (i.e. "take stage #0 of that path"), we should allow ":$n:<path>" to interpret them the same way. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-12-07get_sha1: support relative path ":path" syntaxLibravatar Nguyễn Thái Ngọc Duy1-0/+62
Currently :path and ref:path can be used to refer to a specific object in index or ref respectively. "path" component is absolute path. This patch allows "path" to be written as "./path" or "../path", which is relative to user's original cwd. This does not work in commands for which startup_info is NULL (i.e. non-builtin ones, it seems none of them needs this anyway). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-24rev-parse: tests git rev-parse --verify master@{n}, for various nLibravatar Jon Seymour1-0/+9
This commit introduces tests that verify that rev-parse parses master@{n} correctly for various values of n less than, equal to and greater than the number of revisions in the reference log. In particular, these tests check that rev-parse exits with a non-zero status code and prints a message of the following form to stderr. fatal: Log for [^ ]* only has [0-9][0-9]* entries. Signed-off-by: Jon Seymour <jon.seymour@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-12-07Detailed diagnosis when parsing an object name fails.Libravatar Matthieu Moy1-0/+69
The previous error message was the same in many situations (unknown revision or path not in the working tree). We try to help the user as much as possible to understand the error, especially with the sha1:filename notation. In this case, we say whether the sha1 or the filename is problematic, and diagnose the confusion between relative-to-root and relative-to-$PWD confusion precisely. The 7 new error messages are tested. Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>