kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
brian m. carlson	bd981d5fc3	t/test-lib: introduce OID_REGEX Currently we have a variable, $_x40, which contains a regex that matches a full 40-character hex constant. However, with NewHash, we'll have object IDs that are longer than 40 characters. In such a case, $_x40 will be a confusing name. Create a $OID_REGEX variable which will always reflect a regex matching the appropriate object ID, regardless of the length of the current hash. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
brian m. carlson	8125a58b91	t: switch $_z40 to $ZERO_OID Switch all uses of $_z40 to $ZERO_OID so that they work correctly with larger hashes. This commit was created by using the following sed command to modify all files in the t directory except t/test-lib.sh: sed -i 's/\$_z40/$ZERO_OID/g' Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
brian m. carlson	198857bf7e	t/test-lib: introduce ZERO_OID Currently we have a variable, $_z40, which contains the all-zero object ID. However, with NewHash, we'll have an all-zero object ID which is longer than 40 hex characters. In such a case, $_z40 will be a confusing name. Create a $ZERO_OID variable which will always reflect the all-zeros object ID, regardless of the length of the current hash. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
brian m. carlson	d16ab63429	t/test-lib: add an SHA1 prerequisite There are some basic tests in our codebase that test that we get fixed SHA-1 values. These are valuable because they make sure that our SHA-1 implementation is free of bugs, but obviously these tests will fail with a different hash. There are also tests which intentionally produce objects that have collisions when truncated to a certain length to test our handling of these cases. These tests, too, will fail with a different hash. Add an SHA1 prerequisite to annotate both of these types of tests and disable them when we're using a different hash. In the future, we will create versions of these tests which handle both SHA-1 and NewHash. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Luis Marsano	786ef50a23	git-credential-netrc: accept gpg option git-credential-netrc was hardcoded to decrypt with 'gpg' regardless of the gpg.program option. This is a problem on distributions like Debian that call modern GnuPG something else, like 'gpg2'. Set the command according to these settings in descending precedence 1. the git-credential-netrc command -g\|--gpg option 2. the git gpg.program configuration option 3. the default: 'gpg' For conformance with Documentation/CodingGuidelines - use Git.pm for repository and global option queries - document -g\|--gpg command option in command usage - test repository & command options - write documentation placeholders according to main standards Signed-off-by: Luis Marsano <luis.marsano@gmail.com> Acked-by: Ted Zlatanov <tzz@lifelogs.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Luis Marsano	f07eeed123	git-credential-netrc: adapt to test framework for git git-credential-netrc tests did not run in a test repository. Reuse the main test framework to stage a temporary repository. To imitate Perl tests under t/ - switch to Test::More module - use File::Basename & File::Spec::Functions Signed-off-by: Luis Marsano <luis.marsano@gmail.com> Acked-by: Ted Zlatanov <tzz@lifelogs.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
René Scharfe	7484cf538e	t5512: run git fetch inside test Do the preparatory fetch inside the test of ls-remote --symref to avoid cluttering the test output and to be able to catch unexpected fetch failures. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	8702b30fd7	mark_parents_uninteresting(): avoid most allocation Commit `941ba8db57` (Eliminate recursion in setting/clearing marks in commit list, 2012-01-14) used a clever double-loop to avoid allocations for single-parent chains of history. However, it did so only when following parents of parents (which was an uncommon case), and _always_ incurred at least one allocation to populate the list of pending parents in the first place. We can turn this into zero-allocation in the common case by iterating directly over the initial parent list, and then following up on any pending items we might have discovered. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	43fc643b75	mark_parents_uninteresting(): replace list with stack The mark_parents_uninteresting() function uses two loops: an outer one to process our queue of pending parents, and an inner one to process first-parent chains. This is a clever optimization from `941ba8db57` (Eliminate recursion in setting/clearing marks in commit list, 2012-01-14) to limit the number of linked-list allocations when following single-parent chains. Unfortunately, this makes the result a little hard to read. Let's replace the list with a stack. Then we don't have to worry about doing this double-loop optimization, as we'll just reuse the top element of the stack as we pop/push. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	577dd0d29b	mark_parents_uninteresting(): drop missing object check We allow UNINTERESTING objects in a traversal to be unavailable. As part of this, mark_parents_uninteresting() checks whether we have a particular uninteresting parent; if not, we will mark it as "parsed" so that later code skips it. This code is redundant and even a little bit harmful, so let's drop it. It's redundant because when our parse_object() call in add_parents_to_list() fails, we already quietly skip UNINTERESTING parents. This redundancy is a historical artifact. The mark_parents_uninteresting() protection is from `454fbbcde3` (git-rev-list: allow missing objects when the parent is marked UNINTERESTING, 2005-07-10). Much later, `aeeae1b771` (revision traversal: allow UNINTERESTING objects to be missing, 2009-01-27) covered more cases by making the actual parse more gentle. As an aside, even if this weren't redundant, it would be insufficient. The gentle parsing handles both missing and corrupted objects, whereas the has_object_file() check we're getting rid of covers only missing ones. And the code we're dropping is harmful for two reasons: 1. We spend extra time on the object lookup, even though we don't actually need the information at this point (and will just repeat that lookup later when we parse for the common case that we _do_ have the object). 2. It "lies" about the commit by setting the parsed flag, even though we didn't load any useful data into the struct. This shouldn't matter for the UNINTERESTING case, but we may later clear our flags and do another traversal in the same process. While pretty unlikely, it's possible that we could then look at the same commit without the UNINTERESTING flag, in which case we'd produce the wrong result (we'd think it's a commit with no parents, when in fact we should probably die due to the missing object). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	a5411df0d9	mark_tree_contents_uninteresting(): drop missing object check It's generally acceptable for UNINTERESTING objects in a traversal to be unavailable (e.g., see `aeeae1b771`). When marking trees UNINTERESTING, we access the object database twice: once to check if the object is missing (and return quietly if it is), and then again to actually parse it. We can instead just try to parse; if that fails, we can then return quietly. That halves the effort we spend on locating the object. Note that this isn't _exactly_ the same as the original behavior, as the parse failure could be due to other problems than a missing object: it could be corrupted, in which case the original code would have died. But the new behavior is arguably better, as it covers the object being unavailable for any reason. We'll also still issue a warning to stderr in such a case. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Nguyễn Thái Ngọc Duy	25f859fdf4	commit.h: rearrange 'index' to shrink struct commit On linux 64-bit architecture, pahole finds that there's a 4 bytes padding after 'index'. Moving it to the end reduces this struct's size from 72 to 64 bytes (because of another 4 bytes padding after graph_pos). On linux 32-bit, the struct size remains 52 bytes like before. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ben Peart	e8b2dc2c2a	add status config and command line options for rename detection After performing a merge that has conflicts git status will, by default, attempt to detect renames which causes many objects to be examined. In a virtualized repo, those objects do not exist locally so the rename logic triggers them to be fetched from the server. This results in the status call taking hours to complete on very large repos vs seconds with this patch. Add a new config status.renames setting to enable turning off rename detection during status and commit. This setting will default to the value of diff.renames. Add a new config status.renamelimit setting to to enable bounding the time spent finding out inexact renames during status and commit. This setting will default to the value of diff.renamelimit. Add --no-renames command line option to status that enables overriding the config setting from the command line. Add --find-renames[=<n>] command line option to status that enables detecting renames and optionally setting the similarity index. Reviewed-by: Elijah Newren <newren@gmail.com> Original-Patch-by: Alejandro Pauly <alpauly@microsoft.com> Signed-off-by: Ben Peart <Ben.Peart@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Nguyễn Thái Ngọc Duy	b5d5a567fb	column: fix off-by-one default width By default we want to fill the whole screen if possible, but we do not want to use up _all_ terminal columns because the last character is going hit the border, push the cursor over and wrap. Keep it at default value zero, which will make print_columns() set the width at term_columns() - 1. This affects the test in t7004 because effective column width before was 40 but now 39 so we need to compensate it by one or the output at 39 columns has a different layout. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	be11f7ad60	pager: set COLUMNS to term_columns() After we invoke the pager, our stdout goes to a pipe, not the terminal, meaning we can no longer use an ioctl to get the terminal width. For that reason, `ad6c3739a3` (pager: find out the terminal width before spawning the pager, 2012-02-12) started caching the terminal width. But that cache is only an in-process variable. Any programs we spawn will also not be able to run that ioctl, but won't have access to our cache. They'll end up falling back to our 80-column default. You can see the problem with: git tag --column=row Since git-tag spawns a pager these days, its spawned git-column helper will see neither the terminal on stdout nor a useful COLUMNS value (assuming you do not export it from your shell already). And you'll end up with 80-column output in the pager, regardless of your terminal size. We can fix this by setting COLUMNS right before spawning the pager. That fixes this case, as well as any more complicated ones (e.g., a paged program spawns another script which then generates columnized output). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	db0210d445	refs: handle zero oid for pseudorefs According to the documentation, it is possible to "specify 40 '0' or an empty string as <oldvalue> to make sure that the ref you are creating does not exist." But in the code for pseudorefs, we do not implement this, as demonstrated by the failing tests added in the previous commit. If we fail to read the old ref, we immediately die. But a failure to read would actually be a good thing if we have been given the zero oid. With the zero oid, allow -- and even require -- the ref-reading to fail. This implements the "make sure that the ref ... does not exist" part of the documentation and fixes both failing tests from the previous commit. Since we have a `strbuf err` for collecting errors, let's use it and signal an error to the caller instead of dying hard. Reported-by: Rafael Ascensão <rafa.almas@gmail.com> Helped-by: Rafael Ascensão <rafa.almas@gmail.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	65eb8fc344	t1400: add tests around adding/deleting pseudorefs I have not been able to find any tests around adding pseudorefs using `git update-ref`. Add some as outlined in this table (original design by Michael Haggerty; modified and extended by me): Pre-update value \| ref-update old OID \| Expected result -------------------\|----------------------\|---------------- missing \| value \| reject missing \| none given \| accept set \| none given \| accept set \| correct value \| accept set \| wrong value \| reject missing \| zero \| accept * set \| zero \| reject * The tests marked with a * currently fail, despite git-update-ref(1) claiming that it is possible to "specify 40 '0' or an empty string as <oldvalue> to make sure that the ref you are creating does not exist." These failing tests will be fixed in the next commit. It is only natural to test deletion as well. Test deletion without an old OID, with a correct one and with an incorrect one. Suggested-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	c0bdd658bd	refs.c: refer to "object ID", not "sha1", in error messages We have two error messages that complain about the "sha1". Because we are about to touch one of these sites and add some tests, let's first modernize the messages to say "object ID" instead. While at it, make the second one use `error()` instead of `warning()`. After printing the message, we do not continue, but actually drop the lock and return -1 without deleting the pseudoref. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Nguyễn Thái Ngọc Duy	011b648646	pack-format.txt: more details on pack file format The current document mentions OBJ_* constants without their actual values. A git developer would know these are from cache.h but that's not very friendly to a person who wants to read this file to implement a pack file parser. Similarly, the deltified representation is not documented at all (the "document" is basically patch-delta.c). Translate that C code to English with a bit more about what ofs-delta and ref-delta mean. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	5cc044e025	get_short_oid: sort ambiguous objects by type, then SHA-1 Change the output emitted when an ambiguous object is encountered so that we show tags first, then commits, followed by trees, and finally blobs. Within each type we show objects in hashcmp() order. Before this change the objects were only ordered by hashcmp(). The reason for doing this is that the output looks better as a result, e.g. the v2.17.0 tag before this change on "git show e8f2" would display: hint: The candidates are: hint: `e8f2093055` tree hint: `e8f21caf94` commit 2013-06-24 - bash prompt: print unique detached HEAD abbreviated object name hint: `e8f21d02f7` blob hint: `e8f21d577c` blob hint: `e8f25a3a50` tree hint: e8f26250fa commit 2017-02-03 - Merge pull request #996 from jeffhostetler/jeffhostetler/register_rename_src hint: `e8f2650052` tag v2.17.0 hint: `e8f2867228` blob hint: e8f28d537c tree hint: e8f2a35526 blob hint: e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for multiple remote.url entries hint: e8f2cf6ec0 tree Now we'll instead show: hint: `e8f2650052` tag v2.17.0 hint: `e8f21caf94` commit 2013-06-24 - bash prompt: print unique detached HEAD abbreviated object name hint: e8f26250fa commit 2017-02-03 - Merge pull request #996 from jeffhostetler/jeffhostetler/register_rename_src hint: e8f2bc0c06 commit 2015-05-10 - Documentation: note behavior for multiple remote.url entries hint: `e8f2093055` tree hint: `e8f25a3a50` tree hint: e8f28d537c tree hint: e8f2cf6ec0 tree hint: `e8f21d02f7` blob hint: `e8f21d577c` blob hint: `e8f2867228` blob hint: e8f2a35526 blob Since we show the commit data in the output that's nicely aligned once we sort by object type. The decision to show tags before commits is pretty arbitrary. I don't want to order by object_type since there tags come last after blobs, which doesn't make sense if we want to show the most important things first. I could display them after commits, but it's much less likely that we'll display a tag, so if there is one it makes sense to show it prominently at the top. A note on the implementation: Derrick rightly pointed out[1] that we're bending over backwards here in get_short_oid() to first de-duplicate the list, and then emit it, but could simply do it in one step. The reason for that is that oid_array_for_each_unique() doesn't actually require that the array be sorted by oid_array_sort(), it just needs to be sorted in some order that guarantees that all objects with the same ID are adjacent to one another, which (barring a hash collision, which'll be someone else's problem) the sort_ambiguous() function does. I agree that would be simpler for this code, and had forgotten why I initially wrote it like this[2]. But on further reflection I think it's better to do more work here just so we're not underhandedly using the oid-array API where we lie about the list being sorted. That would break any subsequent use of oid_array_lookup() in subtle ways. I could get around that by hacking the API itself to support this use-case and documenting it, which I did as a WIP patch in [3], but I think it's too much code smell just for this one call site. It's simpler for the API to just introduce a oid_array_for_each() function to eagerly spew out the list without sorting or de-duplication, and then do the de-duplication and sorting in two passes. 1. https://public-inbox.org/git/20180501130318.58251-1-dstolee@microsoft.com/ 2. https://public-inbox.org/git/876047ze9v.fsf@evledraar.gmail.com/ 3. https://public-inbox.org/git/874ljrzctc.fsf@evledraar.gmail.com/ Helped-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	ee930754d8	apply: clarify "-p" documentation We're not really removing slashes, but slash-separated path components. Let's make that more clear. Reported-by: kelly elton <its.the.doc@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
SZEDER Gábor	f6b82970aa	t5516-fetch-push: fix broken &&-chain `b2dc968e60` (t5516: refactor oddball tests, 2008-11-07) accidentaly broke the &&-chain in the test 'push does not update local refs on failure', but since it was in a subshell, chain-lint couldn't notice it. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
SZEDER Gábor	cfb482b6c3	t5516-fetch-push: fix 'push with dry-run' test In a while-at-it cleanup replacing a 'cd dir && <...> && cd ..' with a subshell, commit `28391a80a9` (receive-pack: allow deletion of corrupt refs, 2007-11-29) also moved the assignment of the $old_commit variable to that subshell. This variable, however, is used outside of that subshell as a parameter of check_push_result(), to check that a ref still points to the commit where it is supposed to. With the variable remaining unset outside the subshell check_push_result() doesn't perform that check at all. Use 'git -C <dir> cmd...', so we don't need to change directory, and thus don't need the subshell either when setting $old_commit. Furthermore, change check_push_result() to require at least three parameters (the repo, the oid, and at least one ref), so it will catch similar issues earlier should they ever arise. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	a885c93b5c	sha1-name.c: move around the collect_ambiguous() function A subsequent change will make use of this static function in the get_short_oid() function, which is defined above where the collect_ambiguous() function is now. Without this we'd then have a compilation error due to a forward declaration. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
SZEDER Gábor	87a6bb701a	t5310-pack-bitmaps: make JGit tests work with GIT_TEST_SPLIT_INDEX The two JGit tests 'we can read jgit bitmaps' and 'jgit can read our bitmaps' in 't5310-pack-bitmaps.sh' fail when run with GIT_TEST_SPLIT_INDEX=YesPlease. Both tests create a clone of the test repository to check bitmap interoperability with JGit. With split indexes enabled the index in the clone repositories contains the 'link' extension, which JGit doesn't support and, consequently, an exception aborts it: <...> org.eclipse.jgit.api.errors.JGitInternalException: DIRC extension 'link' not supported by this version. at org.eclipse.jgit.dircache.DirCache.readFrom(DirCache.java:562) <...> Since testing bitmaps doesn't need a worktree in the first place, let's just create bare clones for the two JGit tests, so the cloned won't have an index, and these two tests can be executed even with split index enabled. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	89f32a92b4	git-p4: change "commitish" typo to "committish" This was the only occurrence of "commitish" in the tree, but as the log will reveal we've had others in the past. Fixes up code added in `00ad6e3182` ("git-p4: work with a detached head", 2015-11-21). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	7248672947	sha1-array.h: align function arguments The arguments weren't lined up with the opening parenthesis, after `910650d2` ("Rename sha1_array to oid_array", 2017-03-31) renamed the function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	a264f229cb	sha1-name.c: remove stray newline This stray newline was accidentally introduced in `d2b7d9c7ed` ("sha1_name: convert disambiguate_hint_fn to take object_id", 2017-03-26). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Antonio Ospite	f7997e3682	doc: fix config API documentation about config_with_options In commit `dc8441fdb` ("config: don't implicitly use gitdir or commondir", 2017-06-14) the function git_config_with_options was renamed to config_with_options to better reflect the fact that it does not access the git global config or the repo config by default. However Documentation/technical/api-config.txt still refers to the previous name, fix that. While at it also update the documentation about the extra parameters, because they too changed since the initial definition. Signed-off-by: Antonio Ospite <ao2@ao2.it> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Stefan Beller	74fd0705bb	replace-object.c: remove the_repository from prepare_replace_object This was missed in `5982da9d2c` (replace-object: allow prepare_replace_object to handle arbitrary repositories, 2018-04-11) Technically the code works correctly as the replace_map is the same size in different repositories, however it is hard to read. So convert the code to the familiar pattern of dereferencing the pointer that we assign in the sizeof itself. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Stefan Beller	d587307205	object.c: free replace map in raw_object_store_clear The replace map for objects was missed to free in the object store in the conversion of `174774cd51` (Merge branch 'sb/object-store-replace', 2018-05-08) Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Nguyễn Thái Ngọc Duy	74373b5f10	repository: fix free problem with repo_clear(the_repository) the_repository is special. One of the special things about it is that it does not allocate a new index_state object like submodules but points to the global the_index variable instead. As a global variable, the_index cannot be free()'d. Add an exception for this in repo_clear(). In the future perhaps we would be able to allocate the_repository's index on heap too. Then we can revert this. the_repository->index remains pointed to a clean the_index even after repo_clear() so that it could still be used next time (e.g. in a crazy use case where a dev switches repo in the same process). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
René Scharfe	c112084af9	fast-export: avoid NULL pointer arithmetic Clang 6 reports the following warning, which is turned into an error in a DEVELOPER build: builtin/fast-export.c:162:28: error: performing pointer arithmetic on a null pointer has undefined behavior [-Werror,-Wnull-pointer-arithmetic] return ((uint32_t )NULL) + mark; ~~~~~~~~~~~~~~~~~~ ^ 1 error generated. The compiler is correct, and the error message speaks for itself. There is no need for any undefined operation -- just cast mark to void or uint32_t after an intermediate cast to uintptr_t. That encodes the integer value into a pointer and later decodes it as intended. While at it remove an outdated comment -- intptr_t has been used since `ffe659f94d` (parse-options: make some arguments optional, add callbacks), committed in October 2007. Signed-off-by: Rene Scharfe <l.s.r@web.de> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ramsay Jones	746ea4adc6	BUG_exit_code: fix sparse "symbol not declared" warning Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	0fa5a2ed8d	lock_file: move static locks into functions Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after `076aa2cbd` (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack. (This applies even if a user returns early, leaving a locked lock behind.) Each of these `struct lock_file`s is used from within a single function. Move them into the respective functions to make the scope clearer and drop the staticness. For good measure, I have inspected these sites and come to believe that they always release the lock, with the possible exception of bailing out using `die()` or `exit()` or by returning from a `cmd_foo()`. As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that. After this commit, the remaining occurrences of "static struct lock_file" are locks that are used from within different functions. That is, they need to remain static. (Short of more intrusive changes like passing around pointers to non-static locks.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	b227586831	lock_file: make function-local locks non-static Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after `076aa2cbd` (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack. (This applies even if a user returns early, leaving a locked lock behind.) These `struct lock_file`s are local to their respective functions and we can drop their staticness. For good measure, I have inspected these sites and come to believe that they always release the lock, with the possible exception of bailing out using `die()` or `exit()` or by returning from a `cmd_foo()`. As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that. Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	3c6fad4a3f	refs.c: do not die if locking fails in `delete_pseudoref()` After taking the lock we check whether we got it and die otherwise. But since we take the lock using `LOCK_DIE_ON_ERROR`, we would already have died. Considering the choice between dropping the dead code and dropping the flag, let's go for option number three: Drop the flag, write an error instead of dying, then return -1. This function already returns -1 for another error, so the caller (or rather, its callers) should be able to handle this. There is some inconsistency around how we handle errors in this function and elsewhere in this file, but let's take this small step towards gentle error-reporting now and leave the rest for another time. While at it, make the lock non-static and reduce its scope. (Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after `076aa2cbd` (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	010845157a	refs.c: do not die if locking fails in `write_pseudoref()` If we could not take the lock, we add an error to the `strbuf err` and return. However, this code is dead. The reason is that we take the lock using `LOCK_DIE_ON_ERROR`. Drop the flag to allow our more gentle error-handling to actually kick in. We could instead just drop the dead code and die here. But everything is prepared for gently propagating the error, so let's do that instead. There is similar dead code in `delete_pseudoref()`, but let's save that for the next patch. While at it, make the lock non-static. (Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after `076aa2cbd` (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Martin Ågren	75d9a25e1f	t/helper/test-write-cache: clean up lock-handling Die in case writing the index fails, so that the caller can notice (instead of, say, being impressed by how performant the writing is). While at it, note that after opening a lock with `LOCK_DIE_ON_ERROR`, we do not need to worry about whether we succeeded. Also, we can move the `struct lock_file` into the function and drop the staticness. (Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after `076aa2cbd` (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack.) Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
SZEDER Gábor	4ed79d5203	t6050-replace: don't disable stdin for the whole test script The test script 't6050-replace.sh' starts off with redirecting the whole test script's stdin from /dev/null. This redirection has been there since the test script was introduced in `a3e8267225` (replace_object: add a test case, 2009-01-23), but the commit message doesn't explain why it was deemed necessary. AFAICT, it has never been necessary, and t6050 runs just fine and succeeds even without it, not only the current version but past versions as well. Besides being unnecessary, this redirection is also harmful, as it prevents the test helper functions 'test_pause' and 'debug' from working properly in t6050, because we can't enter any commands to the shell and the debugger, respectively. So let's remove that redirection. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ben Peart	6f10a09e0a	merge: pass aggressive when rename detection is turned off Set aggressive flag in git_merge_trees() when rename detection is turned off. This allows read_tree() to auto resolve more cases that would have otherwise been handled by the rename detection. Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Ben Peart <benpeart@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ben Peart	85b460305c	merge: add merge.renames config setting Add the ability to control rename detection for merge via a config setting. This setting behaves the same and defaults to the value of diff.renames but only applies to merge. Reviewed-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Ben Peart <benpeart@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ben Peart	a7152e9d22	merge: update documentation for {merge,diff}.renameLimit Update the documentation to better indicate that the renameLimit setting is ignored if rename detection is turned off via command line options or config settings. Signed-off-by: Ben Peart <benpeart@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	1de70dbd1a	merge-recursive: fix check for skipability of working tree updates The can-working-tree-updates-be-skipped check has had a long and blemished history. The update can be skipped iff: a) The merge is clean b) The merge matches what was in HEAD (content, mode, pathname) c) The target path is usable (i.e. not involved in D/F conflict) Traditionally, we split b into parts: b1) The merged result matches the content and mode found in HEAD b2) The merged target path existed in HEAD Steps a & b1 are easy to check; we have always gotten those right. While it is easy to overlook step c, this was fixed seven years ago with commit `4ab9a157d0` ("merge_content(): Check whether D/F conflicts are still present", 2010-09-20). merge-recursive didn't have a readily available way to directly check step b2, so various approximations were used: * In commit `b2c8c0a762` ("merge-recursive: When we detect we can skip an update, actually skip it", 2011-02-28), it was noted that although the code claimed it was skipping the update, it did not actually skip the update. The code was made to skip it, but used lstat(path, ...) as an approximation to path-was-tracked-in-index-before-merge. * In commit `5b448b8530` ("merge-recursive: When we detect we can skip an update, actually skip it", 2011-08-11), the problem with using lstat was noted. It was changed to the approximation path2 && strcmp(path, path2) which is also wrong. !path2 \|\| strcmp(path, path2) would have been better, but would have fallen short with directory renames. * In `c5b761fb27` ("merge-recursive: ensure we write updates for directory-renamed file", 2018-02-14), the problem with the previous approximation was noted and changed to was_tracked(path) That looks close to what we were trying to answer, but was_tracked() as implemented at the time should have been named is_tracked(); it returned something different than what we were looking for. * To make matters more complex, fixing was_tracked() isn't sufficient because the splitting of b into b1 and b2 is wrong. Consider the following merge with a rename/add conflict: side A: modify foo, add unrelated bar side B: rename foo->bar (but don't modify the mode or contents) In this case, the three-way merge of original foo, A's foo, and B's bar will result in a desired pathname of bar with the same mode/contents that A had for foo. Thus, A had the right mode and contents for the file, and it had the right pathname present (namely, bar), but the bar that was present was unrelated to the contents, so the working tree update was not skippable. Fix this by introducing a new function: was_tracked_and_matches(o, path, &mfi.oid, mfi.mode) and use it to directly check for condition b. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	05cf21eba2	merge-recursive: make "Auto-merging" comment show for other merges Previously, merge_content() would print "Auto-merging" whenever the final content and mode aren't already available from HEAD. There are a few problems with this: 1) There are other code paths doing merges that should probably have the same message printed, in particular rename/rename(2to1) which cannot call into the normal rename logic. 2) If both sides of the merge have modifications, then a content merge is needed. It may turn out that the end result matches one of the sides (because the other only had a subset of the same changes), but the merge was still needed. Currently, the message will not print in that case, though it seems like it should. Move the printing of this message to merge_file_1() in order to address both issues. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	277292d5ae	merge-recursive: fix remainder of was_dirty() to use original index was_dirty() uses was_tracked(), which has been updated to use the original index rather than the current one. However, was_dirty() also had a separate call to cache_file_exists(), causing it to still implicitly use the current index. Update that to instead use index_file_exists(). Also, was_dirty() had a hack where it would mark any file as non-dirty if we simply didn't know its modification time. This was due to using the current index rather than the original index, because D/F conflicts and such would cause unpack_trees() to not copy the modification times from the original index to the current one. Now that we are using the original index, we can dispense with this hack. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	a35edc84bd	merge-recursive: fix was_tracked() to quit lying with some renamed paths In commit `aacb82de3f` ("merge-recursive: Split was_tracked() out of would_lose_untracked()", 2011-08-11), was_tracked() was split out of would_lose_untracked() with the intent to provide a function that could answer whether a path was tracked in the index before the merge. Sadly, it instead returned whether the path was in the working tree due to having been tracked in the index before the merge OR having been written there by unpack_trees(). The distinction is important when renames are involved, e.g. for a merge where: HEAD: modifies path b other: renames b->c In this case, c was not tracked in the index before the merge, but would have been added to the index at stage 0 and written to the working tree by unpack_trees(). would_lose_untracked() is more interested in the in-working-copy-for-either-reason behavior, while all other uses of was_tracked() want just was-it-tracked-in-index-before-merge behavior. Unsplit would_lose_untracked() and write a new was_tracked() function which answers whether a path was tracked in the index before the merge started. This will also affect was_dirty(), helping it to return better results since it can base answers off the original index rather than an index that possibly only copied over some of the stat information. However, was_dirty() will need an additional change that will be made in a subsequent patch. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	c04ba51739	t6046: testcases checking whether updates can be skipped in a merge Add several tests checking whether updates can be skipped in a merge. Also add several similar testcases for where updates cannot be skipped in a merge to make sure that we skip if and only if we should. In particular: * Testcase 1a (particularly 1a-check-L) would have pointed out the problem Linus has been dealing with for year with his merges[1]. * Testcase 2a (particularly 2a-check-L) would have pointed out the problem with my directory-rename-series before it broke master[2]. * Testcases 3[ab] (particularly 3a-check-L) provide a simpler testcase than 12b of t6043 making that one easier to understand. * There are several complementary testcases to make sure we're not just fixing those particular issues while regressing in the opposite direction. * There are also a pair of tests for the special case when a merge results in a skippable update AND the user has dirty modifications to the path. [1] https://public-inbox.org/git/CA+55aFzLZ3UkG5svqZwSnhNk75=fXJRkvU1m_RHBG54NOoaZPA@mail.gmail.com/ [2] https://public-inbox.org/git/xmqqmuya43cs.fsf@gitster-ct.c.googlers.com/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	2f682e21a6	merge-recursive: avoid triggering add_cacheinfo error with dirty mod If a cherry-pick or merge with a rename results in a skippable update (due to the merged content matching what HEAD already had), but the working directory is dirty, avoid trying to refresh the index as that will fail. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Elijah Newren	bd42380ef1	merge-recursive: move more is_dirty handling to merge_content conflict_rename_normal() was doing some handling for dirty files that more naturally belonged in merge_content. Move it, and rename a parameter for clarity while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago

... 3 4 5 6 7 ...

51655 Commits (09427e8366003ce3d99b0b5a84e42416cef35696) All Branches Search

51655 Commits (09427e8366003ce3d99b0b5a84e42416cef35696)

All Branches