summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* repository.h: add comment and clarify repo_set_gitdirnd/remove-ignore-env-fieldNguyễn Thái Ngọc Duy2018-03-231-1/+5
| | | | | | | | | The argument name "optional" may mislead the reader to think this option could be NULL. But it can't be. While at there, document a bit more about struct set_gitdir_args. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* repository: delete ignore_env memberNguyễn Thái Ngọc Duy2018-03-052-11/+0
| | | | | | | | | | | | | | This variable was added because the repo_set_gitdir() was created to cover both submodule and main repos, but these two are initialized a bit differently so ignore_env == 0 means main repo, while ignore_env != 0 is submodules. Since the difference part (env variables) has been moved out of repo_set_gitdir(), this function works the same way for both repo types and ignore_env is not needed anymore. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* sha1_file.c: move delayed getenv(altdb) back to setup_git_env()Nguyễn Thái Ngọc Duy2018-03-054-5/+9
| | | | | | | | | | | | | getenv() is supposed to work on the main repository only. This delayed getenv() code in sha1_file.c makes it more difficult to convert sha1_file.c to a generic object store that could be used by both submodule and main repositories. Move the getenv() back in setup_git_env() where other env vars are also fetched. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* repository.c: delete dead functionsNguyễn Thái Ngọc Duy2018-03-051-25/+0
| | | | | Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* repository.c: move env-related setup code back to environment.cNguyễn Thái Ngọc Duy2018-03-055-25/+79
| | | | | | | | | | | | | | It does not make sense that generic repository code contains handling of environment variables, which are specific for the main repository only. Refactor repo_set_gitdir() function to take $GIT_DIR and optionally _all_ other customizable paths. These optional paths can be NULL and will be calculated according to the default directory layout. Note that some dead functions are left behind to reduce diff noise. They will be deleted in the next patch. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* repository: initialize the_repository in main()Nguyễn Thái Ngọc Duy2018-03-053-6/+16
| | | | | | | | | | | This simplifies initialization of struct repository and anything inside. Easier to read. Easier to add/remove fields. Everything will go through main() common-main.c so this should cover all programs, including t/helper. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Second batch for 2.17Junio C Hamano2018-02-131-0/+77
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tz/doc-show-defaults-to-head'Junio C Hamano2018-02-131-2/+2
|\ | | | | | | | | | | | | Doc update. * tz/doc-show-defaults-to-head: doc: mention 'git show' defaults to HEAD
| * doc: mention 'git show' defaults to HEADtz/doc-show-defaults-to-headTodd Zullinger2018-01-301-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | When 'git show' is called without any object it defaults to HEAD. This has been true since d4ed9793fd ("Simplify common default options setup for built-in log family.", 2006-04-16). The SYNOPSIS suggests that the object argument is required. Clarify that it is not required and note the default. Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ew/svn-branch-segfault-fix'Junio C Hamano2018-02-131-0/+5
|\ \ | | | | | | | | | | | | | | | | | | Workaround for segfault with more recent versions of SVN. * ew/svn-branch-segfault-fix: git-svn: control destruction order to avoid segfault
| * | git-svn: control destruction order to avoid segfaultew/svn-branch-segfault-fixEric Wong2018-01-301-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It seems necessary to control destruction ordering to avoid a segfault with SVN 1.9.5 when using "git svn branch". I've also reported the problem against libsvn-perl to Debian [Bug #888791], but releasing the SVN::Client instance can be beneficial anyways to save memory. ref: https://bugs.debian.org/888791 Tested-by: Todd Zullinger <tmz@pobox.com> Reported-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Eric Wong <e@80x24.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'sg/travis-linux32-sanity'Junio C Hamano2018-02-134-18/+50
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Travis updates. * sg/travis-linux32-sanity: travis-ci: don't fail if user already exists on 32 bit Linux build job travis-ci: don't run the test suite as root in the 32 bit Linux build travis-ci: don't repeat the path of the cache directory travis-ci: use 'set -e' in the 32 bit Linux build job travis-ci: use 'set -x' for the commands under 'su' in the 32 bit Linux build
| * | | travis-ci: don't fail if user already exists on 32 bit Linux build jobsg/travis-linux32-sanitySZEDER Gábor2018-01-301-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The 32 bit Linux build job runs in a Docker container, which lends itself to running and debugging locally, too. Especially during debugging one usually doesn't want to start with a fresh container every time, to save time spent on installing a bunch of dependencies. However, that doesn't work quite smootly, because the script running in the container always creates a new user, which then must be removed every time before subsequent executions, or the build script fails. Make this process more convenient and don't try to create that user if it already exists and has the right user ID in the container, so developers don't have to bother with running a 'userdel' each time before they run the build script. The build job on Travis CI always starts with a fresh Docker container, so this change doesn't make a difference there. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | travis-ci: don't run the test suite as root in the 32 bit Linux buildSZEDER Gábor2018-01-302-6/+26
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Travis CI runs the 32 bit Linux build job in a Docker container, where all commands are executed as root by default. Therefore, ever since we added this build job in 88dedd5e7 (Travis: also test on 32-bit Linux, 2017-03-05), we have a bit of code to create a user in the container matching the ID of the host user and then to run the test suite as this user. Matching the host user ID is important, because otherwise the host user would have no access to any files written by processes running in the container, notably the logs of failed tests couldn't be included in the build job's trace log. Alas, this piece of code never worked, because it sets the variable holding the user name ($CI_USER) in a subshell, meaning it doesn't have any effect by the time we get to the point to actually use the variable to switch users with 'su'. So all this time we were running the test suite as root. Reorganize that piece of code in 'ci/run-linux32-build.sh' a bit to avoid that problematic subshell and to ensure that we switch to the right user. Furthermore, make the script's optional host user ID option mandatory, so running the build accidentally as root will become harder when debugging locally. If someone really wants to run the test suite as root, whatever the reasons might be, it'll still be possible to do so by explicitly passing '0' as host user ID. Finally, one last catch: since commit 7e72cfcee (travis-ci: save prove state for the 32 bit Linux build, 2017-12-27) the 'prove' test harness has been writing its state to the Travis CI cache directory from within the Docker container while running as root. After this patch 'prove' will run as a regular user, so in future build jobs it won't be able overwrite a previously written, still root-owned state file, resulting in build job failures. To resolve this we should manually delete caches containing such root-owned files, but that would be a hassle. Instead, work this around by changing the owner of the whole contents of the cache directory to the host user ID. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | travis-ci: don't repeat the path of the cache directorySZEDER Gábor2018-01-304-6/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some of our 'ci/*' scripts repeat the name or full path of the Travis CI cache directory, and the following patches will add new places using that path. Use a variable to refer to the path of the cache directory instead, so it's hard-coded only in a single place. Pay extra attention to the 32 bit Linux build: it runs in a Docker container, so pass the path of the cache directory from the host to the container in an environment variable. Note that an environment variable passed this way is exported inside the container, therefore its value is directly available in the 'su' snippet even though that snippet is single quoted. Furthermore, use the variable in the container only if it's been assigned a non-empty value, to prevent errors when someone is running or debugging the Docker build locally, because in that case the variable won't be set as there won't be any Travis CI cache. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | travis-ci: use 'set -e' in the 32 bit Linux build jobSZEDER Gábor2018-01-301-10/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The script 'ci/run-linux32-build.sh' running inside the Docker container of the 32 bit Linux build job uses an && chain to break the build if one of the commands fails. This is problematic for two reasons: - The && chain is broken, because there is this in the middle: test -z $HOST_UID || (CI_USER="ci" && useradd -u $HOST_UID $CI_USER) && Luckily it is broken in a way that it didn't lead to false successes. If installing dependencies fails, then the rest of the first && chain is skipped and execution resumes after the || operator. At that point $HOST_UID is still unset, causing 'useradd' to error out with "invalid user ID 'ci'", which in turn causes the second && chain to abort the script and thus break the build. - All other 'ci/*' scripts use 'set -e' to break the build if one of the commands fails. This inconsistency among these scripts is asking for trouble: I forgot about the && chain more than once while working on this patch series. Enable 'set -e' for the whole script and for the commands executed under 'su' as well. While touching every line in the 'su' command block anyway, change their indentation to use a tab instead of spaces. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | travis-ci: use 'set -x' for the commands under 'su' in the 32 bit Linux buildSZEDER Gábor2018-01-301-0/+1
| | | | | | | | | | | | | | | | | | | | Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'nd/list-merge-strategy'Junio C Hamano2018-02-131-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Completion of "git merge -s<strategy>" (in contrib/) did not work well in non-C locale. * nd/list-merge-strategy: completion: fix completing merge strategies on non-C locales
| * | | | completion: fix completing merge strategies on non-C localesnd/list-merge-strategyDuy Nguyen2018-01-261-1/+1
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The anchor string "Available strategies are:" is translatable so __git_list_merge_strategies may fail to collect available strategies from 'git merge' on non-C locales. Force C locale on this command. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jt/long-running-process-doc'Junio C Hamano2018-02-134-48/+61
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Doc updates. * jt/long-running-process-doc: Docs: split out long-running subprocess handshake
| * | | | Docs: split out long-running subprocess handshakejt/long-running-process-docJonathan Tan2018-01-254-48/+61
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Separating out the implementation of the handshake when starting a long-running subprocess (for example, as is done for a clean/smudge filter) was done in commit fa64a2fdbeed ("sub-process: refactor handshake to common function", 2017-07-26), but its documentation still resides in gitattributes. Split out the documentation as well. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jk/daemon-fixes'Junio C Hamano2018-02-134-20/+107
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Assorted fixes to "git daemon". * jk/daemon-fixes: daemon: fix length computation in newline stripping t/lib-git-daemon: add network-protocol helpers daemon: handle NULs in extended attribute string daemon: fix off-by-one in logging extended attributes t/lib-git-daemon: record daemon log t5570: use ls-remote instead of clone for interp tests
| * | | | daemon: fix length computation in newline strippingjk/daemon-fixesJeff King2018-01-252-4/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When git-daemon gets a pktline request, we strip off any trailing newline, replacing it with a NUL. Clients prior to 5ad312bede (in git v1.4.0) would send: git-upload-pack repo.git\n and we need to strip it off to understand their request. After 5ad312bede, we send the host attribute but no newline, like: git-upload-pack repo.git\0host=example.com\0 Both of these are parsed correctly by git-daemon. But if some client were to combine the two: git-upload-pack repo.git\n\0host=example.com\0 we don't parse it correctly. The problem is that we use the "len" variable to record the position of the NUL separator, but then decrement it when we strip the newline. So we start with: git-upload-pack repo.git\n\0host=example.com\0 ^-- len and end up with: git-upload-pack repo.git\0\0host=example.com\0 ^-- len This is arguably correct, since "len" tells us the length of the initial string, but we don't actually use it for that. What we do use it for is finding the offset of the extended attributes; they used to be at len+1, but are now at len+2. We can solve that by just leaving "len" where it is. We don't have to care about the length of the shortened string, since we just treat it like a C string. No version of Git ever produced such a string, but it seems like the daemon code meant to handle this case (and it seems like a reasonable thing for somebody to do in a 3rd-party implementation). Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t/lib-git-daemon: add network-protocol helpersJeff King2018-01-252-1/+58
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All of our git-protocol tests rely on invoking the client and having it make a request of a server. That gives a nice real-world test of how the two behave together, but it doesn't leave any room for testing how a server might react to _other_ clients. Let's add a few test helper functions which can be used to manually conduct a git-protocol conversation with a remote git-daemon: 1. To connect to a remote git-daemon, we need something like "netcat". But not everybody will have netcat. And even if they do, the behavior with respect to half-duplex shutdowns is not portable (openbsd netcat has "-N", with others you must rely on "-q 1", which is racy). Here we provide a "fake_nc" that is capable of doing a client-side netcat, with sane half-duplex semantics. It relies on perl's IO::Socket::INET. That's been in the base distribution since 5.6.0, so it's probably available everywhere. But just to be on the safe side, we'll add a prereq. 2. To help tests speak and read pktline, this patch adds packetize() and depacketize() functions. I've put fake_nc() into lib-git-daemon.sh, since that's really the only server where we'd need to use a network socket. Whereas the pktline helpers may be of more general use, so I've added them to test-lib-functions.sh. Programs like upload-pack speak pktline, but can talk directly over stdio without a network socket. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | daemon: handle NULs in extended attribute stringJeff King2018-01-252-8/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we receive a request with extended attributes after the NUL, we try to write those attributes to the log. We do so with a "%s" format specifier, which will only show characters up to the first NUL. That's enough for printing a "host=" specifier. But since dfe422d04d (daemon: recognize hidden request arguments, 2017-10-16) we may have another NUL, followed by protocol parameters, and those are not logged at all. Let's cut out the attempt to show the whole string, and instead log when we parse individual attributes. We could leave the "extended attributes (%d bytes) exist" part of the log, which in theory could alert us to attributes that fail to parse. But anything we don't parse as a "host=" parameter gets blindly added to the "protocol" attribute, so we'd see it in that part of the log. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | daemon: fix off-by-one in logging extended attributesJeff King2018-01-252-2/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If receive a request like: git-upload-pack /foo.git\0host=localhost we mark the offset of the NUL byte as "len", and then log the bytes after the NUL with a "%.*s" placeholder, using "pktlen - len" as the length, and "line + len + 1" as the start of the string. This is off-by-one, since the start of the string skips past the separating NUL byte, but the adjusted length includes it. Fortunately this doesn't actually read past the end of the buffer, since "%.*s" will stop when it hits a NUL. And regardless of what is in the buffer, packet_read() will always add an extra NUL terminator for safety. As an aside, the git.git client sends an extra NUL after a "host" field, too, so we'd generally hit that one first, not the one added by packet_read(). You can see this in the test output which reports 15 bytes, even though the string has only 14 bytes of visible data. But the point is that even a client sending unusual data could not get us to read past the end of the buffer, so this is purely a cosmetic fix. Reported-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t/lib-git-daemon: record daemon logJeff King2018-01-251-4/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we start git-daemon for our tests, we send its stderr log stream to a named pipe. We synchronously read the first line to make sure that the daemon started, and then dump the rest to descriptor 4. This is handy for debugging test output with "--verbose", but the tests themselves can't access the log data. Let's dump the log into a file, as well, so that future tests can check the log. There are a few subtleties worth calling out here: - we'll continue to send output to descriptor 4 for viewing/debugging, which would imply swapping out "cat" for "tee". But we want to ensure that there's no buffering, and "tee" doesn't have a standard way to ask for that. So we'll use a shell loop around "read" and "printf" instead. That ensures that after a request has been served, the matching log entries will have made it to the file. - the existing first-line shell loop used read/echo. We'll switch to consistently using "read -r" and "printf" to relay data as faithfully as possible. - we open the logfile for append, rather than just output. That makes it OK for tests to truncate the logfile without restarting the daemon (the OS will atomically seek to the end of the file when outputting each line). That allows tests to look at the log without worrying about pollution from earlier tests. Helped-by: Lucas Werkmeister <mail@lucaswerkmeister.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t5570: use ls-remote instead of clone for interp testsJeff King2018-01-251-6/+3
| | |/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We don't actually care about the clone operation here; we just want to know if we were able to actually contact the remote repository. Using ls-remote does that more efficiently, and without us having to worry about managing the tmp.git directory. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'pw/sequencer-in-process-commit'Junio C Hamano2018-02-1311-280/+765
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The sequencer infrastructure is shared across "git cherry-pick", "git rebase -i", etc., and has always spawned "git commit" when it needs to create a commit. It has been taught to do so internally, when able, by reusing the codepath "git commit" itself uses, which gives performance boost for a few tens of percents in some sample scenarios. * pw/sequencer-in-process-commit: sequencer: run 'prepare-commit-msg' hook t7505: add tests for cherry-pick and rebase -i/-p t7505: style fixes sequencer: assign only free()able strings to gpg_sign sequencer: improve config handling t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1 sequencer: try to commit without forking 'git commit' sequencer: load commit related config sequencer: simplify adding Signed-off-by: trailer commit: move print_commit_summary() to libgit commit: move post-rewrite code to libgit Add a function to update HEAD after creating a commit commit: move empty message checks to libgit t3404: check intermediate squash messages
| * | | | sequencer: run 'prepare-commit-msg' hookpw/sequencer-in-process-commitPhillip Wood2018-01-244-19/+61
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 356ee4659b ("sequencer: try to commit without forking 'git commit'", 2017-11-24) forgot to run the 'prepare-commit-msg' hook when creating the commit. Fix this by writing the commit message to a different file and running the hook. Using a different file means that if the commit is cancelled the original message file is unchanged. Also move the checks for an empty commit so the order matches 'git commit'. Reported-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t7505: add tests for cherry-pick and rebase -i/-pPhillip Wood2018-01-243-4/+157
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Check that cherry-pick and rebase call the 'prepare-commit-msg' hook correctly. The expected values for the hook arguments are taken to match the current master branch. I think there is scope for improving the arguments passed so they make a bit more sense - for instance cherry-pick currently passes different arguments depending on whether the commit message is being edited. Also the arguments for rebase could be improved. Commit 7c4188360ac ("rebase -i: proper prepare-commit-msg hook argument when squashing", 2008-10-3) apparently changed things so that when squashing rebase would pass 'squash' as the argument to the hook but that has been lost. I think that it would make more sense to pass 'message' for revert and cherry-pick -x/-s (i.e. cases where there is a new message or the current message in modified by the command), 'squash' when squashing with a new message and 'commit HEAD/CHERRY_PICK_HEAD' otherwise (picking and squashing without a new message). Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t7505: style fixesPhillip Wood2018-01-241-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix the indentation and style of the hook script in preparation for further changes. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | sequencer: assign only free()able strings to gpg_signJohannes Schindelin2017-12-222-1/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The gpg_sign member of the replay_opts structure is of type `char *`, meaning that the sequencer deems the string to which gpg_sign points to be under its custody, i.e. it needs to be free()d by the sequencer. Therefore, let's only assign malloc()ed buffers to it. Reported-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | sequencer: improve config handlingPhillip Wood2017-12-134-73/+61
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous config handling relied on global variables, called git_default_config() even when the key had already been handled by git_sequencer_config() and did not initialize the diff configuration variables. Improve this by: i) loading the default values for message cleanup and gpg signing of commits into struct replay_opts; ii) restructuring the code to return immediately once a key is handled; and iii) calling git_diff_basic_config(). Note that unfortunately it is not possible to return early if the key is handled by git_gpg_config() as it does not indicate to the caller if the key has been handled or not. The sequencer should probably have been calling git_diff_basic_config() before as it creates a patch when there are conflicts. The shell version uses 'diff-tree' to create the patch so calling git_diff_basic_config() should match that. Although 'git commit' calls git_diff_ui_config() I don't think the output of print_commit_summary() is affected by anything that is loaded by that as print_commit_summary() always turns on rename detection so would ignore the value in the user's configuration anyway. The other values loaded by git_diff_ui_config() are about the formatting of patches so are not relevant to print_commit_summary(). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1Phillip Wood2017-11-242-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Now that the sequencer creates commits without forking 'git commit' it does not see an empty commit in these tests which fixes the known breakage. Note that logic for handling KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1 is not removed from lib-submodule-update.sh as it is still used by other tests. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | sequencer: try to commit without forking 'git commit'Phillip Wood2017-11-241-2/+176
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If the commit message does not need to be edited then create the commit without forking 'git commit'. Taking the best time of ten runs with a warm cache this reduces the time taken to cherry-pick 10 commits by 27% (from 282ms to 204ms), and the time taken by 'git rebase --continue' to pick 10 commits by 45% (from 386ms to 212ms) on my computer running linux. Some of greater saving for rebase is because it no longer wastes time creating the commit summary just to throw it away. The code to create the commit is based on builtin/commit.c. It is simplified as it doesn't have to deal with merges and modified so that it does not die but returns an error to make sure the sequencer exits cleanly, as it would when forking 'git commit' Even when not forking 'git commit' the commit message is written to a file and CHERRY_PICK_HEAD is created unnecessarily. This could be eliminated in future. I hacked up a version that does not write these files and just passed an strbuf (with the wrong message for fixup and squash commands) to do_commit() but I couldn't measure any significant time difference when running cherry-pick or rebase. I think eliminating the writes properly for rebase would require a bit of effort as the code would need to be restructured. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | sequencer: load commit related configPhillip Wood2017-11-244-3/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Load default values for message cleanup and gpg signing of commits in preparation for committing without forking 'git commit'. Note that we interpret commit.cleanup=scissors to mean COMMIT_MSG_CLEANUP_SPACE to be consistent with 'git commit' Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | sequencer: simplify adding Signed-off-by: trailerPhillip Wood2017-11-241-5/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add the Signed-off-by: trailer in one place rather than adding it to the message when doing a recursive merge and specifying '--signoff' when running 'git commit'. This means that if there are conflicts when merging with a strategy other than 'recursive' the Signed-off-by: trailer will be added if the user commits the resolution themselves without passing '--signoff' to 'git commit'. It also simplifies the in-process commit that is about to be added to the sequencer. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | commit: move print_commit_summary() to libgitPhillip Wood2017-11-243-119/+133
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move print_commit_summary() from builtin/commit.c to sequencer.c so it can be shared with other commands. The function is modified by changing the last argument to a flag so callers can specify whether they want to show the author date in addition to specifying if this is an initial commit. If the sequencer dies in print_commit_summary() (which can only happen when cherry-picking or reverting) then neither the todo list nor the abort safety file are updated to reflect the commit that was just made. print_commit_summary() can die if: - The commit that was just created cannot be found or parsed. - HEAD cannot be resolved either because some other process is updating it (which is bad news in the middle of a cherry-pick) or because it is corrupt. - log_tree_commit() cannot read some objects. In all those cases dying will leave the sequencer in a sane state for aborting; 'git cherry-pick --abort' will rewind HEAD to the last successful commit before there was a problem with HEAD or the object database. If the user somehow fixes the problem and runs 'git cherry-pick --continue' then the sequencer will try and pick the same commit again which may or may not be what the user wants depending on what caused print_commit_summary() to die. If print_commit_summary() returned an error instead then update_abort_safety_file() would try to resolve HEAD which may or may not be successful. If it is successful then running 'git rebase --abort' would not rewind HEAD to the last successful commit which is not what we want. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | commit: move post-rewrite code to libgitPhillip Wood2017-11-183-41/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move run_rewrite_hook() from bulitin/commit.c to sequencer.c so it can be shared with other commands and add a new function commit_post_rewrite() based on the code in builtin/commit.c that encapsulates rewriting notes and running the post-rewrite hook. Once the sequencer learns how to create commits without forking 'git commit' these functions will be used when squashing commits. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | Add a function to update HEAD after creating a commitPhillip Wood2017-11-183-19/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add update_head_with_reflog() based on the code that updates HEAD after committing in builtin/commit.c that can be called by 'git commit' and other commands. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | commit: move empty message checks to libgitPhillip Wood2017-11-113-80/+91
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Move the functions that check for empty messages from bulitin/commit.c to sequencer.c so they can be shared with other commands. The functions are refactored to take an explicit cleanup mode and template filename passed by the caller. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | t3404: check intermediate squash messagesPhillip Wood2017-11-111-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When there is more than one squash/fixup command in a row check the intermediate messages are correct. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'nd/shared-index-fix'Junio C Hamano2018-02-132-18/+41
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code clean-up. * nd/shared-index-fix: read-cache: don't write index twice if we can't write shared index read-cache.c: move tempfile creation/cleanup out of write_shared_index read-cache.c: change type of "temp" in write_shared_index()
| * | | | | read-cache: don't write index twice if we can't write shared indexnd/shared-index-fixNguyễn Thái Ngọc Duy2018-01-242-2/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In a0a967568e ("update-index --split-index: do not split if $GIT_DIR is read only", 2014-06-13), we tried to make sure we can still write an index, even if the shared index can not be written. We did so by just calling 'do_write_locked_index()' just before 'write_shared_index()'. 'do_write_locked_index()' always at least closes the tempfile nowadays, and used to close or commit the lockfile if COMMIT_LOCK or CLOSE_LOCK were given at the time this feature was introduced. COMMIT_LOCK or CLOSE_LOCK is passed in by most callers of 'write_locked_index()'. After calling 'write_shared_index()', we call 'write_split_index()', which calls 'do_write_locked_index()' again, which then tries to use the closed lockfile again, but in fact fails to do so as it's already closed. This eventually leads to a segfault. Make sure to write the main index only once. [nd: most of the commit message and investigation done by Thomas, I only tweaked the solution a bit] Helped-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | read-cache.c: move tempfile creation/cleanup out of write_shared_indexNguyễn Thái Ngọc Duy2018-01-161-16/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For one thing, we have more consistent cleanup procedure now and always keep errno intact. The real purpose is the ability to break out of write_locked_index() early when mks_tempfile() fails in the next patch. It's more awkward to do it if this mks_tempfile() is still inside write_shared_index(). Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | read-cache.c: change type of "temp" in write_shared_index()Nguyễn Thái Ngọc Duy2018-01-161-9/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This local variable 'temp' will be passed in from the caller in the next patch. To reduce patch noise, let's change its type now while it's still a local variable and get all the trival conversion out of the next patch. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'po/http-push-error-message'Junio C Hamano2018-02-131-0/+4
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Debugging aid. * po/http-push-error-message: http-push: improve error log
| * | | | | | http-push: improve error logpo/http-push-error-messagePatryk Obara2018-01-241-0/+4
| | |_|/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When git push fails due to server-side WebDAV error, it's not easy to point to the main culprit. Additional information about exact cURL error and HTTP server response is helpful for debugging purpose. New error log helped me pinpoint failing test t5540-http-push-webdav to a missing Apache dependency in Fedora 27: https://bugzilla.redhat.com/show_bug.cgi?id=1491151 Signed-off-by: Patryk Obara <patryk.obara@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'po/clang-format-functype-weight'Junio C Hamano2018-02-131-1/+1
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Prevent "clang-format" from breaking line after function return type. * po/clang-format-functype-weight: clang-format: adjust penalty for return type line break