Junio C Hamano <gitster@xxxxxxxxx> writes: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > >> As a maintainer, I am less concerned about the "result today" than I am >> about keeping things easy and effortless to maintain. One of your patches >> accomplishes that. The other one made it into `next`: >> https://github.com/git/git/commit/91a67b86f77 > > I do not think reverting it and requeuing > > https://lore.kernel.org/git/20201107221916.1428757-1-dgurney99@xxxxxxxxx/ > > would help future folks why we ignore _MSC_VER as any sign usable to > detect endianness, so I'd prefer to see a patch *on top* of 1af265f0 > (compat/bswap.h: simplify MSVC endianness detection, 2020-11-08), > which is 91a67b86f77^2, that explains why we prefer to list archs > explicitly in its log message, which would be the primary value of > that commit. > > Something along this line, perhaps? > > -- >8 -- > > Subject: compat/bswap.h: do not assume MSVC is little-endian only > > Earlier, with 1af265f0 (compat/bswap.h: simplify MSVC endianness > detection, 2020-11-08), we tried to simplify endianness detection > used in compat/bswap.h by assuming that any version Git compiled by > MSVC (detected by _MSC_VER preprocessor macro) is meant to run on > little endian boxes, as the versions of old MSVC that support m68k > and MIPS do not support some C99 features used in the codebase > anyway. > > While it might hold true that modern versions of Windows are all > little-endian, MSVC is and/or can be ported to build for big-endian > boxes, so tying _MSC_VER with endianness is a bit too restrictive. > > Let's go back to the old way to use _MSC_VER to learn what > preprocessor macros compiler uses to tell us which arch we are > building for, and list these arches that are little-endian > explicitly. > > ... signed-off-by from you and helped-by from others ... > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > --- > diffstat > patch Daniel's patch adds _M_ARM64 to the list, but do we need to do anything further to tell the endian on such a bi-endian arch, or does MSVC only support little-endian for that architecture? Just double-checking as the "confusion" that started this thread came from an assumption that MSVC == Windows == big-endian, and you told us MSVC != Windows. Now the patch assumes ARM64-on-MSVC is little-endian only and we want to make sure that assumption is true. And perhaps it is worth documenting in the log, perhaps ... that are little-endian explicitly. Note that ARM64 is bi-endian in nature but we treat it little-endian as MSVC does not treat the arch as bi-endian. or something like that at the end (I do not know what MSVC actually does---just illustrating the level of details I expect in the explanation). Thanks.
