Hi Ramsay, On Fri, 4 May 2018, Ramsay Jones wrote:
> On 04/05/18 07:40, Johannes Schindelin wrote: > [snip] > > BTW I ran `make sparse` for the first time, and it spits out tons of > > stuff. And I notice that they are all non-fatal warnings, but so were the > > ones you pointed out above. This is a bit sad, as I would *love* to > > install a VSTS build job to run `make sparse` automatically. Examples of > > warnings *after* applying your patch: > > > > connect.c:481:40: warning: incorrect type in argument 2 (invalid types) > > connect.c:481:40: expected union __CONST_SOCKADDR_ARG [usertype] __addr > > connect.c:481:40: got struct sockaddr *ai_addr > > > > or > > > > pack-revindex.c:65:23: warning: memset with byte count of 262144 > > > > What gives? > > My stock answer, until recently, was that you are using a very > old version of sparse. Sure. I did this in an Ubuntu 16.04 LTS VM, via `sudo apt-get install sparse`. > Which is probably still true here - but I recently noticed that more > up-to-date platforms/gcc versions also have many problems. (The main > sparse contributors tend to stick with conservative distros and/or don't > use sparse on any software that uses system headers - thus they tend not > to notice the problems caused by new gcc/glibc versions! ;-) ) > > Since I am on Linux Mint 18.3 (based on the last Ubuntu LTS) and build > sparse from source, the current 'master', 'next' and 'pu' branches are > all 'sparse-clean' for me. (On cygwin, which is built with NO_REGEX, I > have a single sparse warning). > > I was just about to say that, unusually for me, I was using a sparse > built from a release tag, but then remembered that I have some > additional patches which fixes a problem on fedora 27! Using sparse on > fedora 27 is otherwise useless. (There are still many warnings spewed on > f27 - but they are caused by incorrect system headers :( ). > > The current release of sparse is v0.5.2, which probably hasn't been > included in any distro yet (I think the previous release v0.5.1, which > should also work for you, is in Debian unstable). If you wanted to try > building a more up-to-date sparse, the repo is at: > git://git.kernel.org/pub/scm/devel/sparse/sparse.git. Well, what I would want to do is let the cloud work for me. By adding an automated build to my Visual Studio Team Services (VSTS) account, of course, as I have "cloud privilege" (i.e. I work in the organization providing the service, so I get to play with all of it for free). So I really don't want to build sparse every time a new revision needs to be tested (whether that be from one of my branches, an internal PR for pre-review of patches to be sent to the mailing list, or maybe even `pu` or the personalized branches on https://github.com/gitster/git). I really would need a ready-to-install sparse, preferably as light-weight as possible (by not requiring any dependencies outside what is available in VSTS' hosted Linux build agents. Maybe there is a specific apt source for sparse? > Linux Mint 19, which will be released in about a month, will be > using the Ubuntu 18.04 LTS as a base, so I guess it is possible > that I will need to debug sparse again ... :-) > BTW, I spent some time last night playing with 'git-branch-diff'. Great! > First of all - Good Job! This tool will be very useful (thanks > also go to Thomas, of course). Both Thomases. Thomas Rast and Thomas Gummerer. > I noticed that there seemed to be an occasional 'whitespace error' > indicator (red background) directly after an +/- change character > which I suspect is an error (I haven't actually checked). However, > this indicator disappears if you add the --dual-color option. Indeed. This is a quirk of the whitespace error paired with diffing diffs: the whitespace error correctly treats the leading space as marker for a context line, but if you diff diffs, the next character may still be a marker for a context line (but this time the "inner" diff). And our whitespace error detection mechanism cannot guess that it looks at a diff of diffs. However, in dual-color mode, we know that we will almost certainly look at diffs of diffs (except if the change is in the commit message, in which case I don't care about whitespace errors at all, anyway). So I have this hack in 16/18: https://public-inbox.org/git/b99ab186c4f11239a10950b9902d9c87d0e60513.1525448066.git.johannes.schinde...@gmx.de/T/#u Essentially, I extend the dual-color mode to detect where such a bogus whitespace error would be detected, and simply *skip the space*! I can get away with that because dual-color is meant for human consumption, and if a horizontal tab follows, it would not matter whether there was a space: it would find the same tab stop. Likewise, if the space comes before a CR or LF, or even just before the final NUL, the space can be safely omitted from the output, too. Ciao, Dscho
