aboutsummaryrefslogtreecommitdiff
path: root/rerere.c
Commit message (Collapse)AuthorAge
* Merge branch 'jk/write-in-full-fix'Junio C Hamano2017-09-25
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many codepaths did not diagnose write failures correctly when disks go full, due to their misuse of write_in_full() helper function, which have been corrected. * jk/write-in-full-fix: read_pack_header: handle signed/unsigned comparison in read result config: flip return value of store_write_*() notes-merge: use ssize_t for write_in_full() return value pkt-line: check write_in_full() errors against "< 0" convert less-trivial versions of "write_in_full() != len" avoid "write_in_full(fd, buf, len) != len" pattern get-tar-commit-id: check write_in_full() return against 0 config: avoid "write_in_full(fd, buf, len) < len" pattern
| * avoid "write_in_full(fd, buf, len) != len" patternJeff King2017-09-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The return value of write_in_full() is either "-1", or the requested number of bytes[1]. If we make a partial write before seeing an error, we still return -1, not a partial value. This goes back to f6aa66cb95 (write_in_full: really write in full or return error on disk full., 2007-01-11). So checking anything except "was the return value negative" is pointless. And there are a couple of reasons not to do so: 1. It can do a funny signed/unsigned comparison. If your "len" is signed (e.g., a size_t) then the compiler will promote the "-1" to its unsigned variant. This works out for "!= len" (unless you really were trying to write the maximum size_t bytes), but is a bug if you check "< len" (an example of which was fixed recently in config.c). We should avoid promoting the mental model that you need to check the length at all, so that new sites are not tempted to copy us. 2. Checking for a negative value is shorter to type, especially when the length is an expression. 3. Linus says so. In d34cf19b89 (Clean up write_in_full() users, 2007-01-11), right after the write_in_full() semantics were changed, he wrote: I really wish every "write_in_full()" user would just check against "<0" now, but this fixes the nasty and stupid ones. Appeals to authority aside, this makes it clear that writing it this way does not have an intentional benefit. It's a historical curiosity that we never bothered to clean up (and which was undoubtedly cargo-culted into new sites). So let's convert these obviously-correct cases (this includes write_str_in_full(), which is just a wrapper for write_in_full()). [1] A careful reader may notice there is one way that write_in_full() can return a different value. If we ask write() to write N bytes and get a return value that is _larger_ than N, we could return a larger total. But besides the fact that this would imply a totally broken version of write(), it would already invoke undefined behavior. Our internal remaining counter is an unsigned size_t, which means that subtracting too many byte will wrap it around to a very large number. So we'll instantly begin reading off the end of the buffer, trying to write gigabytes (or petabytes) of data. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | rerere: allow approxidate in gc.rerereResolved/gc.rerereUnresolvedJunio C Hamano2017-08-22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | These two configuration variables are described in the documentation to take an expiry period expressed in the number of days: gc.rerereResolved:: Records of conflicted merge you resolved earlier are kept for this many days when 'git rerere gc' is run. The default is 60 days. gc.rerereUnresolved:: Records of conflicted merge you have not resolved are kept for this many days when 'git rerere gc' is run. The default is 15 days. There is no strong reason not to allow a more general "approxidate" expiry specification, e.g. "5.days.ago", or "never". Rename the config_get_expiry() helper introduced in the previous step to git_config_get_expiry_in_days() and move it to a more generic place, config.c, and use date.c::parse_expiry_date() to do so. Give it an ability to allow the caller to tell among three cases (i.e. there is no "gc.rerereResolved" config, there is and it is correctly parsed into the *expiry variable, and there was an error in parsing the given value). The current caller can work correctly without using the return value, though. In the future, we may find other variables that only allow an integer that specifies "this many days" or other unit of time, and when it happens we may need to drop "_days" suffix from the name of the function and instead pass the "scale" value as another parameter. But this will do for now. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | rerere: represent time duration in timestamp_t internallyJunio C Hamano2017-08-22
|/ | | | | | | | | | | | | | | The two configuration variables, gc.rerereResolved and gc.rerereUnresolved, are measured in days and are passed as such into the prune_one() helper function, which worked in time_t to see if an entry in the rerere database is past its expiry. Instead, have the caller turn the number of days into the expiry timestamp. Further, use timestamp_t instead of time_t. This will make it possible to extend the way the configuration variable is spelled by using date.c::parse_expiry_date() that gives the expiry timestamp in timestamp_t. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'ab/free-and-null'Junio C Hamano2017-06-24
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | A common pattern to free a piece of memory and assign NULL to the pointer that used to point at it has been replaced with a new FREE_AND_NULL() macro. * ab/free-and-null: *.[ch] refactoring: make use of the FREE_AND_NULL() macro coccinelle: make use of the "expression" FREE_AND_NULL() rule coccinelle: add a rule to make "expression" code use FREE_AND_NULL() coccinelle: make use of the "type" FREE_AND_NULL() rule coccinelle: add a rule to make "type" code use FREE_AND_NULL() git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
| * *.[ch] refactoring: make use of the FREE_AND_NULL() macroÆvar Arnfjörð Bjarmason2017-06-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace occurrences of `free(ptr); ptr = NULL` which weren't caught by the coccinelle rule. These fall into two categories: - free/NULL assignments one after the other which coccinelle all put on one line, which is functionally equivalent code, but very ugly. - manually spotted occurrences where the NULL assignment isn't right after the free() call. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'bw/config-h'Junio C Hamano2017-06-24
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | Fix configuration codepath to pay proper attention to commondir that is used in multi-worktree situation, and isolate config API into its own header file. * bw/config-h: config: don't implicitly use gitdir or commondir config: respect commondir setup: teach discover_git_directory to respect the commondir config: don't include config.h by default config: remove git_config_iter config: create config.h
| * config: don't include config.h by defaultBrandon Williams2017-06-15
| | | | | | | | | | | | | | | | Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | rerere.c: move error_errno() closer to the source system callNguyễn Thái Ngọc Duy2017-05-26
| | | | | | | | | | | | | | We are supposed to report errno from fopen(). fclose() between fopen() and the report function could either change errno or reset it. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | print errno when reporting a system call errorNguyễn Thái Ngọc Duy2017-05-26
| | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | wrapper.c: add and use fopen_or_warn()Nguyễn Thái Ngọc Duy2017-05-26
|/ | | | | | | | | | When fopen() returns NULL, it could be because the given path does not exist, but it could also be some other errors and the caller has to check. Add a wrapper so we don't have to repeat the same error check everywhere. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* hold_locked_index(): align error handling with hold_lockfile_for_update()Junio C Hamano2016-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Callers of the hold_locked_index() function pass 0 when they want to prepare to write a new version of the index file without wishing to die or emit an error message when the request fails (e.g. somebody else already held the lock), and pass 1 when they want the call to die upon failure. This option is called LOCK_DIE_ON_ERROR by the underlying lockfile API, and the hold_locked_index() function translates the paramter to LOCK_DIE_ON_ERROR when calling the hold_lock_file_for_update(). Replace these hardcoded '1' with LOCK_DIE_ON_ERROR and stop translating. Callers other than the ones that are replaced with this change pass '0' to the function; no behaviour change is intended with this patch. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Among the callers of hold_locked_index() that passes 0: - diff.c::refresh_index_quietly() at the end of "git diff" is an opportunistic update; it leaks the lockfile structure but it is just before the program exits and nobody should care. - builtin/describe.c::cmd_describe(), builtin/commit.c::cmd_status(), sequencer.c::read_and_refresh_cache() are all opportunistic updates and they are OK. - builtin/update-index.c::cmd_update_index() takes a lock upfront but we may end up not needing to update the index (i.e. the entries may be fully up-to-date), in which case we do not need to issue an error upon failure to acquire the lock. We do diagnose and die if we indeed need to update, so it is OK. - wt-status.c::require_clean_work_tree() IS BUGGY. It asks silence, does not check the returned value. Compare with callsites like cmd_describe() and cmd_status() to notice that it is wrong to call update_index_if_able() unconditionally.
* cache: convert struct cache_entry to use struct object_idbrian m. carlson2016-09-07
| | | | | | | | | | | | | | | | | | | | | Convert struct cache_entry to use struct object_id by applying the following semantic patch and the object_id transforms from contrib, plus the actual change to the struct: @@ struct cache_entry E1; @@ - E1.sha1 + E1.oid.hash @@ struct cache_entry *E1; @@ - E1->sha1 + E1->oid.hash Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jc/rerere-multi'Junio C Hamano2016-05-23
|\ | | | | | | | | | | * jc/rerere-multi: rerere: remove an null statement rerere: plug memory leaks upon "rerere forget" failure
| * rerere: remove an null statementJunio C Hamano2016-05-19
| | | | | | | | | | | | | | J6t spotted that previous commit added an empty statement by mistake. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: plug memory leaks upon "rerere forget" failureJunio C Hamano2016-05-11
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'nd/error-errno'Junio C Hamano2016-05-17
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The code for warning_errno/die_errno has been refactored and a new error_errno() reporting helper is introduced. * nd/error-errno: (41 commits) wrapper.c: use warning_errno() vcs-svn: use error_errno() upload-pack.c: use error_errno() unpack-trees.c: use error_errno() transport-helper.c: use error_errno() sha1_file.c: use {error,die,warning}_errno() server-info.c: use error_errno() sequencer.c: use error_errno() run-command.c: use error_errno() rerere.c: use error_errno() and warning_errno() reachable.c: use error_errno() mailmap.c: use error_errno() ident.c: use warning_errno() http.c: use error_errno() and warning_errno() grep.c: use error_errno() gpg-interface.c: use error_errno() fast-import.c: use error_errno() entry.c: use error_errno() editor.c: use error_errno() diff-no-index.c: use error_errno() ...
| * | rerere.c: use error_errno() and warning_errno()Nguyễn Thái Ngọc Duy2016-05-09
| | | | | | | | | | | | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'jc/rerere-multi'Junio C Hamano2016-04-25
|\ \ \ | |/ / |/| / | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git rerere" can encounter two or more files with the same conflict signature that have to be resolved in different ways, but there was no way to record these separate resolutions. * jc/rerere-multi: rerere: adjust 'forget' to multi-variant world order rerere: split code to call ll_merge() further rerere: move code related to "forget" together rerere: gc and clear rerere: do use multiple variants t4200: rerere a merge with two identical conflicts rerere: allow multiple variants to exist rerere: delay the recording of preimage rerere: handle leftover rr-cache/$ID directory and postimage files rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_id rerere: split conflict ID further
| * rerere: adjust 'forget' to multi-variant world orderJunio C Hamano2016-04-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because conflicts with the same contents inside conflict blocks enclosed by "<<<<<<<" and ">>>>>>>" can now have multiple variants to help three-way merge to adjust to the differences outside the conflict blocks, "rerere forget $path" needs to be taught that there may be multiple recorded resolutions that share the same conflict hash (which groups the conflicts with "the same contents inside conflict blocks"), among which there are some that would not be relevant to the conflict we are looking at. These "other variants" that happen to share the same conflict hash should not be cleared, and the variant that would apply to the current conflict may not be the zero-th one (which is the only one that is cleared by the current code). After finding the conflict hash, iterate over the existing variants and try to resolve the conflict using each of them to find the one that "cleanly" resolves the current conflict. That is the one we want to forget and record the preimage for, so that the user can record the corrected resolution. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: split code to call ll_merge() furtherJunio C Hamano2016-04-06
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The merge() helper function is given an existing rerere ID (i.e. the name of the .git/rr-cache/* subdirectory, and the variant number) that identifies one <preimage, postimage> pair, try to see if the conflicted state in the given path can be resolved by using the pair, and if this succeeds, then update the conflicted path with the result in the working tree. To implement rerere_forget() in the multiple variant world, we'd need a helper to do the "see if a <preimage, postimage> pair cleanly resolves a conflicted state we have in-core" part, without actually touching any file in the working tree, in order to identify which variant(s) to remove. Split the logic to do so into a separate helper function try_merge() out of merge(). Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: move code related to "forget" togetherJunio C Hamano2016-04-06
| | | | | | | | | | | | | | | | | | "rerere forget" is the only user of handle_cache() helper, which in turn is the only user of rerere_io that reads from an in-core buffer whose getline method is implemented as rerere_mem_getline(). Gather them together. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: gc and clearJunio C Hamano2016-04-06
| | | | | | | | | | | | | | | | Adjust "git rerere gc" and "git rerere clear" to the new world order with rerere database with multiple variants for the same shape of conflicts. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: do use multiple variantsJunio C Hamano2016-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | This enables the multiple-variant support for real. Multiple conflicts of the same shape can have differences in contexts where they appear, interfering the replaying of recorded resolution of one conflict to another, and in such a case, their resolutions are recorded as different variants under the same conflict ID. We still need to adjust garbage collection codepaths for this change, but the basic "replay" functionality is functional with this change. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: allow multiple variants to existJunio C Hamano2016-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The shape of the conflict in a path determines the conflict ID. The preimage and postimage pair that was recorded for the conflict ID previously may or may not replay well for the conflict we just saw. Currently, we punt when the previous resolution does not cleanly replay, but ideally we should then be able to record the currently conflicted path by assigning a new 'variant', and then record the resolution the user is going to make. Introduce a mechanism to have more than one variant for a given conflict ID; we do not actually assign any variant other than 0th variant yet at this step. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: delay the recording of preimageJunio C Hamano2016-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We record the preimage only when there is no directory to record the conflict we encountered, i.e. when $GIT_DIR/rr-cache/$ID does not exist. As the plan is to allow multiple <preimage,postimage> pairs as variants for the same conflict ID eventually, this logic needs to go. As the first step in that direction, stop the "did we create the directory? Then we record the preimage" logic. Instead, we record if a preimage does not exist when we saw a conflict in a path. Also make sure that we remove a stale postimage, which most likely is totally unrelated to the resolution of this new conflict, when we create a new preimage under $ID when $GIT_DIR/rr-cache/$ID already exists. In later patches, we will further update this logic to be "do we have <preimage,postimage> pair that cleanly resolve the current conflicts? If not, record a new preimage as a new variant", but that does not happen at this stage yet. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: handle leftover rr-cache/$ID directory and postimage filesJunio C Hamano2016-03-15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If by some accident there is only $GIT_DIR/rr-cache/$ID directory existed, we wouldn't have recorded a preimage for a conflict that is newly encountered, which would mean after a manual resolution, we wouldn't have recorded it by storing the postimage, because the logic used to be "if there is no rr-cache/$ID directory, then we are the first so record the preimage". Instead, record preimage if we do not have one. In addition, if there is only $GIT_DIR/rr-cache/$ID/postimage without corresponding preimage, we would have tried to call into merge() and punted. These would have been a situation frustratingly hard to recover from. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: scan $GIT_DIR/rr-cache/$ID when instantiating a rerere_idJunio C Hamano2016-02-08
| | | | | | | | | | | | | | | | This will help fixing bootstrap corner-case issues, e.g. having an empty $GIT_DIR/rr-cache/$ID directory would fail to record a preimage, in later changes in this series. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: split conflict ID furtherJunio C Hamano2016-02-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The plan is to keep assigning the backward compatible conflict ID based on the hash of the (normalized) text of conflicts, keep using that conflict ID as the directory name under $GIT_DIR/rr-cache/, but allow each conflicted path to use a separate "variant" to record resolutions, i.e. having more than one <preimage,postimage> pairs under $GIT_DIR/rr-cache/$ID/ directory. As the first step in that direction, separate the shared "conflict ID" out of the rerere_id structure. The plan is to keep information per $ID in rerere_dir, that can be shared among rerere_id that is per conflicted path. When we are done with rerere(), which can be directly called from other programs like "git apply", "git commit" and "git merge", the shared rerere_dir structures can be freed entirely, so they are not reference-counted and they are not freed when we release rerere_id's that reference them. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/rerere-xsnprintf'Junio C Hamano2016-02-17
|\ \ | |/ | | | | | | | | | | | | | | | | | | Some calls to strcpy(3) triggers a false warning from static analysers that are less intelligent than humans, and reducing the number of these false hits helps us notice real issues. A few calls to strcpy(3) in "git rerere" that are already safe has been rewritten to avoid false wanings. * jk/rerere-xsnprintf: rerere: replace strcpy with xsnprintf
| * rerere: replace strcpy with xsnprintfJeff King2016-02-08
| | | | | | | | | | | | | | | | | | | | This shouldn't overflow, as we are copying a sha1 hex into a 41-byte buffer. But it does not hurt to use a bound-checking function, which protects us and makes auditing for overflows easier. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jc/rerere'Junio C Hamano2015-10-05
|\ \ | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up and minor fixes. * jc/rerere: (21 commits) rerere: un-nest merge() further rerere: use "struct rerere_id" instead of "char *" for conflict ID rerere: call conflict-ids IDs rerere: further clarify do_rerere_one_path() rerere: further de-dent do_plain_rerere() rerere: refactor "replay" part of do_plain_rerere() rerere: explain the remainder rerere: explain "rerere forget" codepath rerere: explain the primary codepath rerere: explain MERGE_RR management helpers rerere: fix benign off-by-one non-bug and clarify code rerere: explain the rerere I/O abstraction rerere: do not leak mmfile[] for a path with multiple stage #1 entries rerere: stop looping unnecessarily rerere: drop want_sp parameter from is_cmarker() rerere: report autoupdated paths only after actually updating them rerere: write out each record of MERGE_RR in one go rerere: lift PATH_MAX limitation rerere: plug conflict ID leaks rerere: handle conflicts with multiple stage #1 entries ...
| * rerere: un-nest merge() furtherJunio C Hamano2015-07-24
| | | | | | | | | | | | | | By consistently using "upon failure, set 'ret' and jump to out" pattern, flatten the function further. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: use "struct rerere_id" instead of "char *" for conflict IDJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | This gives a thin abstraction between the conflict ID that is a hash value obtained by inspecting the conflicts and the name of the directory under $GIT_DIR/rr-cache/, in which the previous resolution is recorded to be replayed. The plan is to make sure that the presence of the directory does not imply the presense of a previous resolution and vice-versa, and later allow us to have more than one pair of <preimage, postimage> for a given conflict ID. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: call conflict-ids IDsJunio C Hamano2015-07-24
| | | | | | | | | | | | | | Most places we call conflict IDs "name" and some others we call them "hex"; update all of them to "id". Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: further clarify do_rerere_one_path()Junio C Hamano2015-07-24
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: further de-dent do_plain_rerere()Junio C Hamano2015-07-24
| | | | | | | | | | | | It's just easier to follow this way. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: refactor "replay" part of do_plain_rerere()Junio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | Extract the body of a loop that attempts to replay recorded resolution for each conflicted path into a helper function, not because I want to call it from multiple places later, but because the logic has become too deeply nested and hard to read. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: explain the remainderJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | Explain the internals of rerere as in-code comments, while sprinkling "NEEDSWORK" comment to highlight iffy bits and questionable assumptions. This covers the codepath that implements "rerere gc" and "rerere clear". Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: explain "rerere forget" codepathJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | Explain the internals of rerere as in-code comments, while sprinkling "NEEDSWORK" comment to highlight iffy bits and questionable assumptions. This covers the codepath that implements "rerere forget". Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: explain the primary codepathJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | Explain the internals of rerere as in-code comments, while sprinkling "NEEDSWORK" comment to highlight iffy bits and questionable assumptions. This one covers the codepath reached from rerere(), the primary interface to the subsystem. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: explain MERGE_RR management helpersJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | Explain the internals of rerere as in-code comments, while sprinkling "NEEDSWORK" comment to highlight iffy bits and questionable assumptions. This one covers the "$GIT_DIR/MERGE_RR" file and in-core merge_rr that are used to keep track of the status of "rerere" session in progress. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: fix benign off-by-one non-bug and clarify codeJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | rerere_io_putconflict() wants to use a limited fixed-sized buf[] on stack repeatedly to formulate a longer string, but its implementation is doubly confusing: * When it knows that the whole thing fits in buf[], it wants to fill early part of buf[] with conflict marker characters, followed by a LF and a NUL. It miscounts the size of the buffer by 1 and does not use the last byte of buf[]. * When it needs to show only the early part of a long conflict marker string (because the whole thing does not fit in buf[]), it adjusts the number of bytes shown in the current round in a strange-looking way. It makes sure that this round does not emit all bytes and leaves at least one byte to the next round, so that "it all fits" case will pick up the rest and show the terminating LF. While this is correct, one needs to stop and think for a while to realize why it is correct without an explanation. Fix the benign off-by-one, and add comments to explain the strange-looking size adjustment. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: explain the rerere I/O abstractionJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | Explain the internals of rerere as in-code comments. This one covers our thin I/O abstraction to read from either a file or a memory while optionally writing out to a file. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: do not leak mmfile[] for a path with multiple stage #1 entriesJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A conflicted index can have multiple stage #1 entries when dealing with a criss-cross merge and using the "resolve" merge strategy. Plug the leak by reading only the first one of the same stage entries. Strictly speaking, this fix does change the semantics, in that we used to use the last stage #1 entry as the common ancestor when doing the plain-vanilla three-way merge, but with the leak fix, we will use the first stage #1 entry. But it is not a grave backward compatibility breakage. Either way, we are arbitrarily picking one of multiple stage #1 entries and using it, ignoring others, and there is no meaning in the ordering of these stage #1 entries. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: stop looping unnecessarilyJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | handle_cache() loops 3 times starting from an index entry that is unmerged, while ignoring an entry for a path that is different from what we are looking for. As the index is sorted, once we see a different path, we know we saw all stages for the path we are interested in. Just loop while we see the same path and then break, instead of continuing for 3 times. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: drop want_sp parameter from is_cmarker()Junio C Hamano2015-07-24
| | | | | | | | | | | | | | | | As the nature of the conflict marker line determines if there should be a SP and label after it, the caller shouldn't have to pass the parameter redundantly. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: report autoupdated paths only after actually updating themJunio C Hamano2015-07-24
| | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: write out each record of MERGE_RR in one goJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | | | Instead of writing the hash for a conflict, a HT, and the path with three separate write_in_full() calls, format them into a single record into a strbuf and write it out in one go. As a more recent "rerere remaining" codepath abuses the .util field of the merge_rr data to store a sentinel token, make sure that codepath does not call into this function (of course, "remaining" is a read-only operation and currently does not call it). Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * rerere: lift PATH_MAX limitationJunio C Hamano2015-07-24
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The MERGE_RR file records a collection of NUL-terminated entries, each of which consists of - a hash that identifies the conflict - a HT - the pathname We used to read this piece-by-piece, and worse yet, read the pathname part a byte at a time into a fixed buffer of size PATH_MAX. Instead, read a whole entry using strbuf_getwholeline() and parse out the fields. This way, we issue fewer read(2) calls and more importantly we do not have to limit the pathname to PATH_MAX. Signed-off-by: Junio C Hamano <gitster@pobox.com>