Re: Feature request: hooks directory
Hello Christian, 2018-09-03 6:00 GMT+02:00 Christian Couder : > Hi Wesley, > > On Sun, Sep 2, 2018 at 11:38 PM, Wesley Schwengle wrote: >> Hi all, >> >> I've made some progress with the hook.d implementation. It isn't >> finished, as it is my first C project I'm still somewhat rocky with >> how pointers and such work, but I'm getting somewhere. I haven't >> broken any tests \o/. > > Great! Welcome to the Git community! Thank you! >> You have a nice testsuite btw. Feel free to comment on the code. >> >> I've moved some of the hooks-code found in run-command.c to hooks.c >> >> You can see the progress on gitlab: https://gitlab.com/waterkip/git >> or on github: https://github.com/waterkip/git >> The output of format-patch is down below. > > It would be nicer if you could give links that let us see more > directly the commits you made, for example: > https://gitlab.com/waterkip/git/commits/multi-hooks Yeah.. sorry about that. Let's just say I was excited to send my progress to the list. >> I have some questions regarding the following two functions in run-command.c: >> * run_hook_le >> * run_hook_ve >> >> What do the postfixes le and ve mean? > > It's about the arguments the function accepts, in a similar way to > exec*() functions, see `man execve` and `man execle`. > In short 'l' means list, 'v' means array of pointer to strings and 'e' > means environment. Thanks, I'll have a look at these functions later today. >> format-patch: >> >> From 129d8aff8257b22210beadc155cdbcae99b0fc4b Mon Sep 17 00:00:00 2001 >> From: Wesley Schwengle >> Date: Sun, 2 Sep 2018 02:40:04 +0200 >> Subject: [PATCH] WIP: Add hook.d support in git > > This is not the best way to embed a patch in an email. There is > Documentation/SubmittingPatches in the code base, that should explain > better ways to send patches to the mailing list. I saw that as well, after I've submitted the e-mail and was looking at the travis documentation. I'll promise I'll do better for my next patch submission. Sorry about this.. >> Add a generic mechanism to find and execute one or multiple hooks found >> in $GIT_DIR/hooks/ and/or $GIT_DIR/hooks/.d/* >> >> [snip] >> >> * All the scripts are executed and fail on the first error > > Maybe the above documentation should be fully embedded as comments in > "hooks.h" (or perhaps added to a new file in > "Documentation/technical/", though it looks like we prefer to embed > doc in header files these days). I've added this to "hooks.h". If you guys want some documentation in "Documentation/technical", I'm ok with adding a new file there as well. >> diff --git a/hooks.h b/hooks.h >> new file mode 100644 >> index 0..278d63344 >> --- /dev/null >> +++ b/hooks.h >> @@ -0,0 +1,35 @@ >> +#ifndef HOOKS_H >> +#define HOOKS_H >> + >> +#ifndef NO_PTHREADS >> +#include >> +#endif >> +/* >> + * Returns all the hooks found in either >> + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/ >> + * >> + * Note that this points to static storage that will be >> + * overwritten by further calls to find_hooks and run_hook_*. >> + */ >> +//extern const struct string_list *find_hooks(const char *name); > > The above comment is using "//" which we forbid and should probably be > removed anyway. Thanks, I have a "//" comment elsewhere, I'll change/remove it. >> +extern const char *find_hooks(const char *name); >> + >> +/* Unsure what this does/is/etc */ >> +//LAST_ARG_MUST_BE_NULL > > This is to make it easier for tools like compilers to check that a > function is used correctly. You should not remove such macros. Check. >> +/* >> + * Run all the runnable hooks found in >> + * $GIT_DIR/hooks/$hook and/or $GIT_DIR/hooks/$hook.d/ >> + * >> + */ >> +//extern int run_hooks_le(const char *const *env, const char *name, ...); >> +//extern int run_hooks_ve(const char *const *env, const char *name, >> va_list args); > > Strange that these functions are commented out. These two functions are still in "run-command.h" and I want to move them to "hooks.h" and friends. But I first wanted to make sure "find_hooks" worked as intended. This is still on my TODO for this week. >> +#endif >> + >> +/* Workings of hooks >> + * >> + * array_of_hooks = find_hooks(name); >> + * number_of_hooks_ran = run_hooks(array_of_hooks); >> + * >> + */ > > This kind of documentation should probably be at the beginning of the > file, see strbuf.h for example. Since I added the better part of the commit message in "hooks.h" I removed this bit. An additional question: In my patch I've added "hooks.multiHook", which I think I should rename to "hooks.hooksd". It is wanted to change "core.hooksPath" to the "hooks" namespace? Or should I rename my new config item to "core.hooksd"? Cheers, Wesley -- Wesley Schwengle, Developer Mintlab B.V., https://www.zaaksysteem.nl E: wes...@mintlab.nl T: +31 20 737 00 05
Re: Feature request: hooks directory
Hi all, I've made some progress with the hook.d implementation. It isn't finished, as it is my first C project I'm still somewhat rocky with how pointers and such work, but I'm getting somewhere. I haven't broken any tests \o/. You have a nice testsuite btw. Feel free to comment on the code. I've moved some of the hooks-code found in run-command.c to hooks.c You can see the progress on gitlab: https://gitlab.com/waterkip/git or on github: https://github.com/waterkip/git The output of format-patch is down below. I have some questions regarding the following two functions in run-command.c: * run_hook_le * run_hook_ve What do the postfixes le and ve mean? Cheers, Wesley format-patch: >From 129d8aff8257b22210beadc155cdbcae99b0fc4b Mon Sep 17 00:00:00 2001 From: Wesley Schwengle Date: Sun, 2 Sep 2018 02:40:04 +0200 Subject: [PATCH] WIP: Add hook.d support in git Add a generic mechanism to find and execute one or multiple hooks found in $GIT_DIR/hooks/ and/or $GIT_DIR/hooks/.d/* The API is as follows: #include "hooks.h" array hooks = find_hooks('pre-receive'); int hooks_ran = run_hooks(hooks); The implemented behaviour is: * If we find a hooks/.d directory and the hooks.multiHook flag isn't set we make use of that directory. * If we find a hooks/.d and we also have hooks/ and the hooks.multiHook isn't set or set to false we don't use the hook.d directory. If the hook isn't set we issue a warning to the user telling him/her that we support multiple hooks via the .d directory structure * If the hooks.multiHook is set to true we use the hooks/ and all the entries found in hooks/.d * All the scripts are executed and fail on the first error --- Makefile | 1 + TODO-hooks.md | 38 builtin/am.c | 4 +- builtin/commit.c | 4 +- builtin/receive-pack.c | 10 +-- builtin/worktree.c | 3 +- cache.h| 1 + config.c | 5 ++ environment.c | 1 + hooks.c| 134 + hooks.h| 35 +++ run-command.c | 36 +-- run-command.h | 6 -- sequencer.c| 7 ++- transport.c| 3 +- 15 files changed, 237 insertions(+), 51 deletions(-) create mode 100644 TODO-hooks.md create mode 100644 hooks.c create mode 100644 hooks.h diff --git a/Makefile b/Makefile index 5a969f583..f5eddf1d7 100644 --- a/Makefile +++ b/Makefile @@ -810,6 +810,7 @@ LIB_H = $(shell $(FIND) . \ -name Documentation -prune -o \ -name '*.h' -print) +LIB_OBJS += hooks.o LIB_OBJS += abspath.o LIB_OBJS += advice.o LIB_OBJS += alias.o diff --git a/TODO-hooks.md b/TODO-hooks.md new file mode 100644 index 0..13b15bffb --- /dev/null +++ b/TODO-hooks.md @@ -0,0 +1,38 @@ +# All hooks +# See Documentation/githooks.txt for more information about each and every hook +# that git knows about +commit-msg +fsmoninor-watchman +p4-pre-submit +post-applypatch +post-checkout +post-commit +post-merge +post-receive +post-rewrite +post-update +pre-applypatch +pre-auto-gc +pre-commit +pre-push +pre-rebase +pre-receive +prepare-commit-msg +push-to-checkout +sendemail-validate +update + +# builtin/receive-pack.c +feed_recieve_hook +find_hook +find_receive_hook +push_to_checkout_hook +receive_hook_feed_state +run_and_feed_hook +run_hook_le +run_receive_hook +run_update_hook + + +# run-command.c +find_hook diff --git a/builtin/am.c b/builtin/am.c index 9f7ecf6ec..d1276dd47 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -34,6 +34,8 @@ #include "packfile.h" #include "repository.h" +#include "hooks.h" + /** * Returns 1 if the file is empty or does not exist, 0 otherwise. */ @@ -509,7 +511,7 @@ static int run_applypatch_msg_hook(struct am_state *state) static int run_post_rewrite_hook(const struct am_state *state) { struct child_process cp = CHILD_PROCESS_INIT; - const char *hook = find_hook("post-rewrite"); + const char *hook = find_hooks("post-rewrite"); int ret; if (!hook) diff --git a/builtin/commit.c b/builtin/commit.c index 0d9828e29..389224d66 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -34,6 +34,8 @@ #include "mailmap.h" #include "help.h" +#include "hooks.h" + static const char * const builtin_commit_usage[] = { N_("git commit [] [--] ..."), NULL @@ -932,7 +934,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (!no_verify && find_hook("pre-commit")) { + if (!no_verify && find_hooks("pre-commit")) { /* * Re-read the index as pre-commit hook could have updated it, * and write it out as a tree. We must do this before we invoke diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index c17ce94e1..dd
Re: Feature request: hooks directory
Hop, 2018-08-30 16:45 GMT+02:00 Ævar Arnfjörð Bjarmason : >> Solution: >> We discussed this at work and we thought about making a .d directory >> for the hooks, eg. $GIT_DIR/hooks/post-commit.d, where a user can put >> the post-commit hooks in. This allows us to provide post commit hooks >> and allows the user to add additional hooks him/herself. We could >> implement this in our own code base. But we were wondering if this >> approach could be shared with the git community and if this behavior >> is wanted in git itself. > > There is interest in this. This E-Mail of mine gives a good summary of > prior discussions about this: > https://public-inbox.org/git/877eqqnq22@evledraar.gmail.com/ > > I.e. it's something I've personally been interested in doing in the > past, there's various bolt-on solutions to do it (basically local hook > runners) used by various projects. Thank you for the input. Do you by any chance still have that branch? Or would you advise me to start fresh, if so, do you have any pointers on where to look as I'm brand new to the git source code? >From the thread I've extracted three stories: 1) As a developer I want to have 'hooks.multiHooks' to enable multi-hook support in git Input is welcome for another name. 2) As a developer I want natural sort order executing for my githooks so I can predict executions See https://public-inbox.org/git/CACBZZX6AYBYeb5S4nEBhYbx1r=icJ81JGYBx5=h4wacphhj...@mail.gmail.com/ for more information 3) As a developer I want to run $GIT_DIR/hooks/ before $GIT_DIR/hooks/.d/* Reference: https://public-inbox.org/git/cacbzzx6j6q2dun_z-pnent1u714dvnpfbrl_pieqylmczlu...@mail.gmail.com/ The following story would be.. nice to have I think. I'm not sure I would want to implement this from the get go as I don't have a use case for it. 4) As a developer I want a way to have a hook report an error and let another hook decide if we want to pass or not. Reference: https://public-inbox.org/git/xmqq60v4don1@gitster.mtv.corp.google.com/ 2018-08-31 5:16 GMT+02:00 Jonathan Nieder : > A few unrelated thoughts, to expand on this. > > Separately from that, in [1] I mentioned that I want to revamp how > hooks work somewhat, to avoid the attack described there (or the more > common attack also described there that involves a zip file). Such a > revamp would be likely to also handle this multiple-hook use case. > > [1] > https://public-inbox.org/git/20171002234517.gv19...@aiede.mtv.corp.google.com/ The zip file attack vector doesn't change with adding a hook.d directory structure? If I have one file or multiple files, the attack stays the same? I think I'm asking if this would be a show stopper for the feature. Cheers, Wesley -- Wesley Schwengle, Developer Mintlab B.V., https://www.zaaksysteem.nl E: wes...@mintlab.nl T: +31 20 737 00 05
Feature request: hooks directory
Hello all, I would like to ask if it is worth my time looking into the following solution to a problem we have at work. Problem: We want to have some git-hooks and we want to provide them to the user. In a most recent example we have a post-checkout hook that deals with some Docker things. However, if we update that post-checkout hook my local overrides in that post-checkout hook are going to be overwritten. Solution: We discussed this at work and we thought about making a .d directory for the hooks, eg. $GIT_DIR/hooks/post-commit.d, where a user can put the post-commit hooks in. This allows us to provide post commit hooks and allows the user to add additional hooks him/herself. We could implement this in our own code base. But we were wondering if this approach could be shared with the git community and if this behavior is wanted in git itself. Cheers, Wesley -- Wesley Schwengle, Developer Mintlab B.V., https://www.zaaksysteem.nl E: wes...@mintlab.nl T: +31 20 737 00 05
Cześć słodka
Am Wes ze Stanów Zjednoczonych, ale obecnie przebywa w Syrii na misji pokojowej. Obecnie szukam przyjaźni, która doprowadzi do związku, w którym znowu czuję się kochana ... Chcę cię lepiej poznać, jeśli mogę być odważny. Uważam się za łatwego człowieka .. Proszę wybaczyć moje maniery nie są dobre, jeśli chodzi o Internet, ponieważ to nie jest moja dziedzina. Tutaj w Syrii nie wolno nam wychodzić, co sprawia, że bardzo się nudzę, więc myślę, że potrzebuję przyjaciela do rozmowy z zewnątrz, żeby mnie utrzymać ... Chciałbym poznać "prawdziwego" ciebie jako przyjaciela. Twoje polubienia, nielubienia, twoje zainteresowania .. co cię wyróżnia. Mój ulubiony kolor to niebieski. Moje ulubione jedzenie to BACON, mogłem z łatwością zostać wegetarianinem, gdyby nie było to na bekonie !! Mam nadzieję, że możesz mi powiedzieć więcej szczegółów na temat twojej pracy, związku i przeszłości . Mam nadzieję, że wkrótce skontaktuję się z Tobą . Wes.
Cześć słodka
Am Wes ze Stanów Zjednoczonych, ale obecnie przebywa w Syrii na misji pokojowej. Obecnie szukam przyjaźni, która doprowadzi do związku, w którym znowu czuję się kochana ... Chcę cię lepiej poznać, jeśli mogę być odważny. Uważam się za łatwego człowieka .. Proszę wybaczyć moje maniery nie są dobre, jeśli chodzi o Internet, ponieważ to nie jest moja dziedzina. Tutaj w Syrii nie wolno nam wychodzić, co sprawia, że bardzo się nudzę, więc myślę, że potrzebuję przyjaciela do rozmowy z zewnątrz, żeby mnie utrzymać ... Chciałbym poznać "prawdziwego" ciebie jako przyjaciela. Twoje polubienia, nielubienia, twoje zainteresowania .. co cię wyróżnia. Mój ulubiony kolor to niebieski. Moje ulubione jedzenie to BACON, mogłem z łatwością zostać wegetarianinem, gdyby nie było to na bekonie !! Mam nadzieję, że możesz mi powiedzieć więcej szczegółów na temat twojej pracy, związku i przeszłości . Mam nadzieję, że wkrótce skontaktuję się z Tobą . Wes.
Cześć słodka
Am Wes ze Stanów Zjednoczonych, ale obecnie przebywa w Syrii na misji pokojowej. Obecnie szukam przyjaźni, która doprowadzi do związku, w którym znowu czuję się kochana ... Chcę cię lepiej poznać, jeśli mogę być odważny. Uważam się za łatwego człowieka .. Proszę wybaczyć moje maniery nie są dobre, jeśli chodzi o Internet, ponieważ to nie jest moja dziedzina. Tutaj w Syrii nie wolno nam wychodzić, co sprawia, że bardzo się nudzę, więc myślę, że potrzebuję przyjaciela do rozmowy z zewnątrz, żeby mnie utrzymać ... Chciałbym poznać "prawdziwego" ciebie jako przyjaciela. Twoje polubienia, nielubienia, twoje zainteresowania .. co cię wyróżnia. Mój ulubiony kolor to niebieski. Moje ulubione jedzenie to BACON, mogłem z łatwością zostać wegetarianinem, gdyby nie było to na bekonie !! Mam nadzieję, że możesz mi powiedzieć więcej szczegółów na temat twojej pracy, związku i przeszłości . Mam nadzieję, że wkrótce skontaktuję się z Tobą . Wes.
RE: Is finalize_object_file in sha1_file.c handling errno from "rename" correctly?
Junio, Thanks for your response. I'm glad to see that you've been able to understand the problem. I'm working with the Windows git team to properly return EACCESS when "rename" fails due to access permissions, but it also sounds like there will need to be a fix to finalize_object_file to better handle the case of an existing file when renaming. Wesley Smith T: 503.597.0556 | wsm...@greatergiving.com -Original Message- From: Junio C Hamano [mailto:gits...@pobox.com] Sent: Thursday, September 14, 2017 11:51 PM To: Wesley Smith Cc: git@vger.kernel.org Subject: Re: Is finalize_object_file in sha1_file.c handling errno from "rename" correctly? Wesley Smith writes: > 1) This bug is triggered because "git fetch" is causing a pack file to > be written when that same pack file already exists. It seems like > this is harmless and shouldn't cause a problem. Is that correct? The final name of the packfile is derived from the entire contents of the packfile; it should be harmless when we attempt to rename a new file, which has exactly the same contents as an existing file, to the existing file and see a failure out of that attempt. > 2) It seems that finalize_object_file is not accounting for the fact > that "link" will return EEXIST if the destination file already exists > but is not writeable, whereas "rename" will return EACCESS in this > case. Is that correct? If so, should finalize_object_file be fixed > to account for this? Perhaps it should check if the newfile exists > before calling rename. Or, should the Windows mingw_rename function > be modified to return EEXIST in this case, even though that's not the > standard errno for that situation? The codepath that is triggered by OBJECT_CREATION_USES_RENAMES ought to behave correctly even on non-Windows platforms, so bending the error code of rename() only on Windows to fit the existing error handling would not be a smart thing to do. Rather, the rename() emulation should leave a correct errno and the caller should be updated to be aware of that error that is not EEXIST, which it currently knows about. Thanks for spotting a problem and digging to its cause. This is a #leftoverbits tangent, and should be done separately from your "OBJECT_CREATION_USES_RENAMES is broken" topic, but I think it is a bug to use finalize_object_file() directly to "finalize" anything but an individual loose object file in the first place. We should create a new shared helper that does what the function currently does, make finalize_object_file() call that new shared helper, and make sure finalize_object_file() is called only on a newly created loose object file. The codepath that creates a new packfile and other things and moves them to the final name should not call finalize_object_file() but the new shared helper instead. That way, we could later implement the "collision? check" alluded by the in-code comment in finailize_object_file(), and we won't have to worry about affecting callers other than the one that creates a loose object file with such an enhancement.
Is finalize_object_file in sha1_file.c handling errno from "rename" correctly?
Using git 2.14.1 for Windows I'm seeing an issue with the follow sequence of commands: git init D:\XXX\workspace git fetch --no-tags --progress https://XXX/_git/PAPI +refs/heads/*:refs/remotes/origin/* --depth=20 git fetch --no-tags --progress https://XXX/_git/PAPI +refs/heads/*:refs/remotes/origin/* --depth=20 git fetch --no-tags --progress https://XXX/_git/PAPI +refs/heads/*:refs/remotes/origin/* --depth=20 The third "git fetch" command hangs forever and takes 100% of the CPU. I've debugged this a bit, and what I've found is that after the first fetch, the .git/objects/pack directory contains 2 files: pack-b64910484b4254836a6413ce6a94019278fc54c5.pack pack-b64910484b4254836a6413ce6a94019278fc54c5.idx After the second fetch, the directory contains 4 files: pack-b64910484b4254836a6413ce6a94019278fc54c5.pack pack-b64910484b4254836a6413ce6a94019278fc54c5.idx pack-ae983dc9c8057f4d5d2c8cdc3485cb6badde864b.pack pack-ae983dc9c8057f4d5d2c8cdc3485cb6badde864b.idx When the third "git fetch" is run, it spawns this chain of commands: git fetch --no-tags --progress https://XXX /_git/PAPI +refs/heads/*:refs/remotes/origin/* --depth=20 git remote-https https://XXX/_git/PAPI https://XXX/_git/PAPI git-remote-https https://XXX/_git/PAPI https://XXX/_git/PAPI git fetch-pack --stateless-rpc --stdin --lock-pack --thin --depth=20 https://XXX/_git/PAPI/ git --shallow-file D:/XXX/workspace/.git/shallow.lock index-pack --stdin -v --fix-thin "--keep=fetch-pack 15728 on DT0004" --pack_header=2,3425 It's the final of these git instances (the --shallow-file one) that's hanging. Upon debugging this "git --shallow-file" process, the problem is as follows: (line numbers relative to https://github.com/git/git/blob/master/sha1_file.c) In sha1_file.c, finalize_object_file is called with a tmpfile value of "tmp_pack_AmXsya" and a filename of "pack-ae983dc9c8057f4d5d2c8cdc3485cb6badde864b.pack". Note that this filename already exists (it was created by the second fetch). On line 3236, the condition (object_creation_mode == OBJECT_CREATION_USES_RENAMES) is true on Windows, so the code runs the goto try_rename. On line 1378, rename is called, which on Windows is defined as a specialized function called mingw_rename. I've identified a bug in this Windows-specific mingw_rename function that causes an infinite loop if the new filename (pack-ae983dc9c8057f4d5d2c8cdc3485cb6badde864b.pack) already exists, _and_ is locked by another process. In this case, it appears that the first "git fetch" call in the process chain has opened the pack file, which is why this process can't rename the temp file to that name. I can fix the infinite loop in the mingw_rename function, but the question is what errno should be returned by mingw_rename, and that brings me to my question regarding the finalize_object_file function. On UNIX-style OSes, the code would first try to perform a "link" call in line 1380. According to my reading of the link(2) man page, I think (but haven't tested) that link call would return EEXIST in this case (the newpath already exists). If link returns EEXIST, then the code will skip most of the rest of the code in finalize_object_file, and will return 0 (success) on line 1411. However, on systems where object_creation_mode is OBJECT_CREATION_USES_RENAMES, then the code will call "rename" instead on line 1396. According to my reading of the rename(2) man page, EACCES would be returned in this case (because the pack file is locked by another process). Notably, EEXIST would _not_ be returned from rename, as rename only returns EEXIST if "newpath is a nonempty directory". Since finalize_object_file doesn't have any special logic for EACCES, if I fixed the Windows version of the rename function to return the correct errno (EACCES), then the finalize_object_file will return the error "unable to write sha1 filename" on 1403 and that will cause the program to die. My questions: 1) This bug is triggered because "git fetch" is causing a pack file to be written when that same pack file already exists. It seems like this is harmless and shouldn't cause a problem. Is that correct? 2) It seems that finalize_object_file is not accounting for the fact that "link" will return EEXIST if the destination file already exists but is not writeable, whereas "rename" will return EACCESS in this case. Is that correct? If so, should finalize_object_file be fixed to account for this? Perhaps it should check if the newfile exists before calling rename. Or, should the Windows mingw_rename function be modified to return EEXIST in this case, even though that's not the standard errno for that situation? Thanks for your help, Wesley Smith
Hello Beautiful
I hope you are doing well and this email meet you in good health condition. My name is Wesley.I`m from the US but currently in Syria for peace keeping mission. I want to get to know you better, if I may be so bold. I consider myself an easy-going man, and I am currently looking for a relationship in which I feel loved. Please forgive my manners am not good when it comes to Internet because that is not really my field .Here in Syria we are not allowed to go out that makes it very bored for me so I just think I need a friend to talk to outside to keep me going. Please tell me more about yourself, if you don't mind. Hope to hear from you soon. Regards, Wesley.
Hello beautiful, I'm just seeking for serious friendship
My name is Wesley. from the US but currently in Syria for peace keeping mission. I want to get to know you better, if I may be so bold. I consider myself an easy-going man, and I am currently looking for a relationship in which I feel loved. Please forgive my manners am not good when it comes to Internet because that is not really my field .Here in Syria we are not allowed to go out that makes it very bored for me so I just think I need a friend to talk to outside to keep me going. Please tell me more about yourself, if you don't mind. Hope to hear from you soon. Regards, Wesley.
Hello beautiful
How you doing today? I hope you are doing well. My name is Wesley, from the US. I'm in Syria right now fighting ISIS. I want to get to know you better, if I may be so bold. I consider myself an easy-going man, and I am currently looking for a relationship in which I feel loved. Please tell me more about yourself, if you don't mind. Hope to hear from you soon. Regards, Wesley.
Re: How to create the " [PATCH 0/5]" first email?
On Monday, September 17, 2012 17:49:39 Junio C Hamano wrote: > "Philip Oakley" writes: > > I then applied it (using git am) to a temp branch to see what it > > produced, and could repeat the cycle until the patches looked right. > > That's another obvious and valid way to prepare your series. It all > depends on how comfortable you are to directly edit patches. Some > people fear it. Some don't. Some can do it with their eyes closed ;-). > > > However, when it came to creating the series, with comments, I > > couldn't see a way of having my comments within my local commits, but > > preparing a patch series that would properly include the '---' > > separator. > > An unofficial trick that works is to write the > > --- > > * This is an additional comment > > > yourself when running "git commit". That will be propagated to the > output from format-patch. You will have another "---" in front of > the diffstat, but nobody is hurt by that. One thing I have done is to add the additional comments I want with "git notes", then give the "--notes" option to format-patch or send-email. Unfortunately, this sticks the notes right into the commit message section, because the "--notes" option is actually a diff option, not something format-patch knows about, so you have to make sure to manually move it. But even so, I've found it a a nice way to track comments. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation: indent-with-non-tab uses "equivalent tabs" not 8
From: "Wesley J. Landaker" Update the documentation of the core.whitespace option "indent-with-non-tab" to correctly reflect that it catches the use of spaces instead of the equivalent tabs, rather than a fixed number. Signed-off-by: Wesley J. Landaker --- Documentation/config.txt |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6416cae..11f320b 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -559,8 +559,9 @@ core.whitespace:: * `space-before-tab` treats a space character that appears immediately before a tab character in the initial indent part of the line as an error (enabled by default). -* `indent-with-non-tab` treats a line that is indented with 8 or more - space characters as an error (not enabled by default). +* `indent-with-non-tab` treats a line that is indented with space + characters instead of the equivalent tabs as an error (not enabled by + default). * `tab-in-indent` treats a tab character in the initial indent part of the line as an error (not enabled by default). * `blank-at-eof` treats blank lines added at the end of file as an error -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: indent-with-non-tab uses tabwidth setting, not just 8
On Monday, September 17, 2012 00:03:29 Junio C Hamano wrote: > An alternative would be to lose the "8" (or `tabwidth`) from that > description. I've always thought that the description of `tabwidth` > is clear enough that "8" in the patch is not a hardcoded non-overridable > value but is merely a default, but after reading that section a few > more times, I no longer think that is the case. > > I originally wrote "8 or more space" but that wasn't because I > thought it was important to stress "8 is the default", but because I > didn't think of a better way to say what I wanted to say, which was > "if you are filling the indentation with spaces when you could have > just typed a tab with a few spaces, this error triggers", in other > words "use of this is to encourage indenting with tabs". Okay, I'm going to generated a new patch that hopefully reads better! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: indent-with-non-tab uses tabwidth setting, not just 8
On 09/16/2012 11:16 PM, Junio C Hamano wrote: > I would rather see this part left untouched. > > Your new text will force people who are not interested in using > non-standard tab width to read through the bulletted list, only to > find "The default tab width is 8". I think that is a regression in > the documentation for more common readers. > > When somebody wants to use `indent-with-non-tab` and gets offended > by the seemingly hardcoded "8" in the description, the reader has > incentive to find out if there is a way to change that 8, and will > find `tabwidth=` in the same bulletted list described, with the > effect it has on both `indent-with-non-tab` and `tab-in-indent`. > > I think that should be sufficient for people who do use non-standard > tab width using tabwidth=. Well, I'm not going to push the issue further than this e-mail, but I very much disagree. Please think about this: * The whole whitespace section talks generically about "spaces" and "tab characters". All of the options talk about tab in a generic way, with the one single exception of "indent-with-non-tab". * I know all about the tabwidth setting (I have it set in my configuration), but when I went looking in the whitespace documentation to try to flag a certain error I wanted to avoid, I was confused because "indent-with-non-tab" didn't do what I wanted ... instead it apparently used a hard-coded length of 8 spaces. My first thought was, well, I'd better fix THAT bug! * Of course, I did an experiment, and of course, it DOESN'T ACTUALLY DO WHAT THE DOCUMENTATION SAYS, instead it uses the tabwidth. This is good, I'm not complaining about how it works: this *is* what I want it to do. But the documentation is still wrong. * So, as you say, "the reader has incentive to find out if there is a way to change that 8". I did get incentive to find that, but it took me a few minutes of wasted time experimenting around with it, and then motived me to write a patch so that no one else will ever get confused about it again. If I've perhaps convinced you that it would be beneficial to make the documentation for this option precisely correct, but you don't like how it's worded (it's the way it is because I tried to make a very minimal change) I'd be happy to revise the patch, perhaps by changing the order of presentation of the options (e.g. mentioning tab width earlier in the section, or in some other way that you or someone may want to suggest). In any case, please, let's find some way to make the documentation both easy to read and also absolutely correct! =) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Documentation: indent-with-non-tab uses tabwidth setting, not just 8
From: "Wesley J. Landaker" Update the documentation of the core.whitespace option "indent-with-non-tab" to correctly reflect that it uses the currently set tab width, set by the "tabwidth" option, rather than a fixed number. Signed-off-by: Wesley J. Landaker --- Documentation/config.txt |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 6416cae..113a196 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -559,8 +559,8 @@ core.whitespace:: * `space-before-tab` treats a space character that appears immediately before a tab character in the initial indent part of the line as an error (enabled by default). -* `indent-with-non-tab` treats a line that is indented with 8 or more - space characters as an error (not enabled by default). +* `indent-with-non-tab` treats a line that is indented with `tabwidth` space + characters or more as an error (not enabled by default). * `tab-in-indent` treats a tab character in the initial indent part of the line as an error (not enabled by default). * `blank-at-eof` treats blank lines added at the end of file as an error -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html