Re: build deps
David Aguilar venit, vidit, dixit 16.10.2012 03:39: On Mon, Oct 15, 2012 at 1:53 PM, Junio C Hamano gits...@pobox.com wrote: Michael J Gruber g...@drmicha.warpmail.net writes: grep.c:451:16: warning: comparison of unsigned enum expression 0 is always false [-Wtautological-compare] if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field) ^ ~ 1 warning generated. Right, that enum type starts at 0. Junio, you last touched this area. Can we just dump the first comparison or did you have something else in mind? I think it was a leftover from the very first implementation that defensively said this has to be one of these known ones, and tried to bound it from both sides of the range, regaredless of the actual type of the field (these GREP_HEADER_WHAT things may have been simple integers with #define'd values). Dropping the negative comparison is perfectly fine. This snippet of code came up before: http://thread.gmane.org/gmane.comp.version-control.git/184908/focus=185014 There seemed to be good reasons to keep the check at the time. Was this same snippet not also touched when Nguyen Thai Ngoc Duy worked on the even if I'm drunk patch?: http://thread.gmane.org/gmane.comp.version-control.git/206413/focus=206539 With the drunk patch then we wouldn't need the check at all, which is really nice. I hope that helps jog folks' memories. I'm not sure if the above discussions are relevant anymore, but I figured it'd be good to provide some more context. cheers, The drunk patch, cheers ;) That's very valuable context that you are giving. So it's either avoiding the warning and relying and enum unsignedness (or human/static analysis) or playing it safe and keeping the warning. How is if (/* p-field 0 || */ GREP_HEADER_FIELD_MAX = p-field) to remind any reader that the first condition should be granted? One could take this further and use a macro but that seems overkill. Michael -- 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: build deps
Thiago Farina venit, vidit, dixit 12.10.2012 06:08: On Thu, Oct 11, 2012 at 10:06 PM, Andrew Wong andrew.kw.w.li...@gmail.com wrote: On 10/11/12 16:54, Thiago Farina wrote: Just setting CC to gcc works for me. But still, I'd like to be able to build with clang (may be as you noted is just something with the + in my PATH). Oh, I just realized you were using sudo. The PATH environment was probably not inherited when you use sudo to run make. So the subsequent shells statred by make' were not able to find clang. Interesting, thank you for your observation. This worked for me now: $ git clone https://github.com/gitster/git $ cd git $ make configure $ ./configure $ make $ ./git version git version 1.8.0.rc2 clang reported this: combine-diff.c:1006:19: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] prefix = COLONS + offset; ~~~^~~~ combine-diff.c:1006:19: note: use array indexing to silence this warning prefix = COLONS + offset; ^ [ ] 1 warning generated. Does COLONS[offset] silence that? grep.c:451:16: warning: comparison of unsigned enum expression 0 is always false [-Wtautological-compare] if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field) ^ ~ 1 warning generated. Right, that enum type starts at 0. Junio, you last touched this area. Can we just dump the first comparison or did you have something else in mind? Michael -- 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: build deps
On Mon, Oct 15, 2012 at 12:44 PM, Michael J Gruber g...@drmicha.warpmail.net wrote: clang reported this: combine-diff.c:1006:19: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] prefix = COLONS + offset; ~~~^~~~ combine-diff.c:1006:19: note: use array indexing to silence this warning prefix = COLONS + offset; ^ [ ] 1 warning generated. Does COLONS[offset] silence that? Yes. -- 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: build deps
Michael J Gruber g...@drmicha.warpmail.net writes: grep.c:451:16: warning: comparison of unsigned enum expression 0 is always false [-Wtautological-compare] if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field) ^ ~ 1 warning generated. Right, that enum type starts at 0. Junio, you last touched this area. Can we just dump the first comparison or did you have something else in mind? I think it was a leftover from the very first implementation that defensively said this has to be one of these known ones, and tried to bound it from both sides of the range, regaredless of the actual type of the field (these GREP_HEADER_WHAT things may have been simple integers with #define'd values). Dropping the negative comparison is perfectly fine. -- 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: build deps
On Mon, Oct 15, 2012 at 1:53 PM, Junio C Hamano gits...@pobox.com wrote: Michael J Gruber g...@drmicha.warpmail.net writes: grep.c:451:16: warning: comparison of unsigned enum expression 0 is always false [-Wtautological-compare] if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field) ^ ~ 1 warning generated. Right, that enum type starts at 0. Junio, you last touched this area. Can we just dump the first comparison or did you have something else in mind? I think it was a leftover from the very first implementation that defensively said this has to be one of these known ones, and tried to bound it from both sides of the range, regaredless of the actual type of the field (these GREP_HEADER_WHAT things may have been simple integers with #define'd values). Dropping the negative comparison is perfectly fine. This snippet of code came up before: http://thread.gmane.org/gmane.comp.version-control.git/184908/focus=185014 There seemed to be good reasons to keep the check at the time. Was this same snippet not also touched when Nguyen Thai Ngoc Duy worked on the even if I'm drunk patch?: http://thread.gmane.org/gmane.comp.version-control.git/206413/focus=206539 With the drunk patch then we wouldn't need the check at all, which is really nice. I hope that helps jog folks' memories. I'm not sure if the above discussions are relevant anymore, but I figured it'd be good to provide some more context. cheers, -- David -- 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: build deps
On Mon, Oct 8, 2012 at 7:52 PM, Andrew Wong andrew.k...@gmail.com wrote: On 10/08/12 17:36, Thiago Farina wrote: OK, after running ./configure I tried the make command again. CC credential-store.o /bin/sh: clang: not found make: *** [credential-store.o] Error 127 $ which clang /home/tfarina/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang $ clang --version clang version 3.2 (trunk 163674) Target: x86_64-unknown-linux-gnu Thread model: posix Looks like something went wrong with make setting PATH. I wonder if the + sign in your path is somehow messing things up. Would be something that could be fixed in git? Are you trying to compile specifically with clang? Nope, it just happen that I switched to clang because I use it to compile chromium and I need it to use the chrome_plugin[1]. If not, maybe try unsetting the CC env var, and run configure again? Just setting CC to gcc works for me. But still, I'd like to be able to build with clang (may be as you noted is just something with the + in my PATH). [1] http://git.chromium.org/gitweb/?p=chromium.git;a=tree;f=tools/clang/plugins;h=8e79d8f35d5ccfee82b6ab8f27ea8b5d820c772d;hb=HEAD -- 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: build deps
On 10/11/12 16:54, Thiago Farina wrote: Just setting CC to gcc works for me. But still, I'd like to be able to build with clang (may be as you noted is just something with the + in my PATH). Oh, I just realized you were using sudo. The PATH environment was probably not inherited when you use sudo to run make. So the subsequent shells statred by make' were not able to find clang. -- 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: build deps
On Thu, Oct 11, 2012 at 10:06 PM, Andrew Wong andrew.kw.w.li...@gmail.com wrote: On 10/11/12 16:54, Thiago Farina wrote: Just setting CC to gcc works for me. But still, I'd like to be able to build with clang (may be as you noted is just something with the + in my PATH). Oh, I just realized you were using sudo. The PATH environment was probably not inherited when you use sudo to run make. So the subsequent shells statred by make' were not able to find clang. Interesting, thank you for your observation. This worked for me now: $ git clone https://github.com/gitster/git $ cd git $ make configure $ ./configure $ make $ ./git version git version 1.8.0.rc2 clang reported this: combine-diff.c:1006:19: warning: adding 'int' to a string does not append to the string [-Wstring-plus-int] prefix = COLONS + offset; ~~~^~~~ combine-diff.c:1006:19: note: use array indexing to silence this warning prefix = COLONS + offset; ^ [ ] 1 warning generated. grep.c:451:16: warning: comparison of unsigned enum expression 0 is always false [-Wtautological-compare] if (p-field 0 || GREP_HEADER_FIELD_MAX = p-field) ^ ~ 1 warning generated. -- 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: build deps
On Mon, Oct 8, 2012 at 6:36 PM, Thiago Farina tfrans...@gmail.com wrote: On Mon, Oct 8, 2012 at 1:09 PM, Andrew Wong andrew.k...@gmail.com wrote: On 10/07/12 20:39, Thiago Farina wrote: When trying to build from source but it's failing: $ sudo make prefix=/usr/local all LINK git-credential-store gcc: @CHARSET_LIB@: No such file or directory make: *** [git-credential-store] Error 1 Did you run the configure script? Hum, I haven't. Now I did. In the source folder, do you have either file config.mak or config.mak.autogen ? After running ./configure, now I have. If you do, try removing them, and compile again. Which version are you compiling? $ cat GIT-VERSION-FILE GIT_VERSION = 1.7.12.84.gefa6462 Did you get the source files from tar? Or from git? From git (git://git.kernel.org/pub/scm/git/git.git). OK, after running ./configure I tried the make command again. CC credential-store.o /bin/sh: clang: not found make: *** [credential-store.o] Error 127 $ which clang /home/tfarina/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang $ clang --version clang version 3.2 (trunk 163674) Target: x86_64-unknown-linux-gnu Thread model: posix Also: $ echo $CC clang -B/usr/local/gold/bin -- 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: build deps
On 10/08/12 17:36, Thiago Farina wrote: OK, after running ./configure I tried the make command again. CC credential-store.o /bin/sh: clang: not found make: *** [credential-store.o] Error 127 $ which clang /home/tfarina/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang $ clang --version clang version 3.2 (trunk 163674) Target: x86_64-unknown-linux-gnu Thread model: posix Looks like something went wrong with make setting PATH. I wonder if the + sign in your path is somehow messing things up. Are you trying to compile specifically with clang? If not, maybe try unsetting the CC env var, and run configure again? -- 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
