{"thread":{"id":"54598","subject":"[PATCH] compat/bswap.h: Simplify MSVC endianness detection","startedAt":"2020-11-07T23:48:22Z","lastAt":"2020-11-10T22:37:48Z","messageCount":13,"participants":["Daniel Gurney","brian m. carlson","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"409329","messageId":"20201107234751.10661-1-dgurney99@gmail.com","threadId":"54598","inReplyTo":null,"subject":"[PATCH] compat/bswap.h: Simplify MSVC endianness detection","fromName":"Daniel Gurney","fromEmail":"dgurney99@gmail.com","sentAt":"2020-11-07T23:47:51Z","receivedAt":"2020-11-07T23:48:22Z","isPatch":true,"sender":{"key":"dgurney99@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12816807?v=4"},"body":"Modern MSVC or Windows versions don't support big-endian, so it's\nunnecessary to consider architectures when using it.\n\nSigned-off-by: Daniel Gurney \n---\n compat/bswap.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex c0bb744adc..72f225eaa8 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -74,7 +74,7 @@ static inline uint64_t git_bswap64(uint64_t x)\n }\n #endif\n \n-#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))\n+#elif defined(_MSC_VER)\n \n #include \n \n-- \n2.29.2\n\n"},{"id":"409330","messageId":"20201108001254.GG6252@camp.crustytoothpaste.net","threadId":"54598","inReplyTo":"20201107234751.10661-1-dgurney99@gmail.com","subject":"Re: [PATCH] compat/bswap.h: Simplify MSVC endianness detection","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-11-08T00:12:54Z","receivedAt":"2020-11-08T00:13:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-11-07 at 23:47:51, Daniel Gurney wrote:\n> Modern MSVC or Windows versions don't support big-endian, so it's\n> unnecessary to consider architectures when using it.\n> \n> Signed-off-by: Daniel Gurney \n> ---\n> compat/bswap.h | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/compat/bswap.h b/compat/bswap.h\n> index c0bb744adc..72f225eaa8 100644\n> --- a/compat/bswap.h\n> +++ b/compat/bswap.h\n> @@ -74,7 +74,7 @@ static inline uint64_t git_bswap64(uint64_t x)\n> }\n> #endif\n> \n> -#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))\n> +#elif defined(_MSC_VER)\n> \n> #include \n> \n\nThe patch itself seems fine to me, of course. For the message, it may\nbe valuable to mention that this enables the proper behavior for ARM64,\nwhich was your original goal. Also, by convention, we typically use a\nlower case letter after the colon in the summary.\n\nAnd of course, thanks for the patch. I always have a soft spot for\npatches that improve architecture support.\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"409333","messageId":"20201108095739.23144-1-dgurney99@gmail.com","threadId":"54598","inReplyTo":"20201107234751.10661-1-dgurney99@gmail.com","subject":"[PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Daniel Gurney","fromEmail":"dgurney99@gmail.com","sentAt":"2020-11-08T09:57:41Z","receivedAt":"2020-11-08T10:02:59Z","isPatch":true,"sender":{"key":"dgurney99@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12816807?v=4"},"body":"Modern MSVC or Windows versions don't support big-endian, so it's\nunnecessary to consider architectures when using it.\n\nThis also makes ARM64 MSVC builds succeed.\n\nSigned-off-by: Daniel Gurney \n---\nv2: just edited the commit message as suggested by Brian.\n\n compat/bswap.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/compat/bswap.h b/compat/bswap.h\nindex c0bb744adc..72f225eaa8 100644\n--- a/compat/bswap.h\n+++ b/compat/bswap.h\n@@ -74,7 +74,7 @@ static inline uint64_t git_bswap64(uint64_t x)\n }\n #endif\n \n-#elif defined(_MSC_VER) && (defined(_M_IX86) || defined(_M_X64))\n+#elif defined(_MSC_VER)\n \n #include \n \n-- \n2.29.2\n\n"},{"id":"409442","messageId":"20201110003127.GA1268480@coredump.intra.peff.net","threadId":"54598","inReplyTo":"20201108095739.23144-1-dgurney99@gmail.com","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-10T00:31:27Z","receivedAt":"2020-11-10T00:31:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 08, 2020 at 11:57:41AM +0200, Daniel Gurney wrote:\n\n> Modern MSVC or Windows versions don't support big-endian, so it's\n> unnecessary to consider architectures when using it.\n\nThis made me wonder if we support any non-modern versions (which would\nbe negatively impacted).\n\nFrom the earlier thread at [1], it sounds like probably not, but I\nwonder if we can offer a stronger argument there (or just define\n\"modern\" a little more clearly).\n\n-Peff\n\n[1] https://lore.kernel.org/git/20201107224747.GF6252@camp.crustytoothpaste.net/\n"},{"id":"409461","messageId":"20201110023620.GH6252@camp.crustytoothpaste.net","threadId":"54598","inReplyTo":"20201110003127.GA1268480@coredump.intra.peff.net","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-11-10T02:36:20Z","receivedAt":"2020-11-10T02:36:28Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-11-10 at 00:31:27, Jeff King wrote:\n> On Sun, Nov 08, 2020 at 11:57:41AM +0200, Daniel Gurney wrote:\n> \n> > Modern MSVC or Windows versions don't support big-endian, so it's\n> > unnecessary to consider architectures when using it.\n> \n> This made me wonder if we support any non-modern versions (which would\n> be negatively impacted).\n\nI'm pretty sure we don't. As I said, we're using several C99 features\nand that version precedes the C99 standard (and 1999).\n\n> From the earlier thread at [1], it sounds like probably not, but I\n> wonder if we can offer a stronger argument there (or just define\n> \"modern\" a little more clearly).\n\nAccording to Wikipedia[0]:\n\n Visual C++ 2013 [12.0] finally added support for various C99 features\n in its C mode (including designated initializers, compound literals,\n and the _Bool type), though it was still not complete. Visual C++ 2015\n further improved the C99 support, with full support of the C99\n Standard Library, except for features that require C99 language\n features not yet supported by the compiler.\n\nThe version mentioned that supported MIPS, Alpha, and m68k was Visual\nC++ 2.0 RISC Edition. While Wikipedia doesn't mention its release date,\nits successor, Visual C++ 4.0, was released in 1995. The m68k version\nran on Macs using those processors, and Apple abandoned m68k for PowerPC\nin 1994[1].\n\nI'm entirely comfortable with requiring that people use a compiler and\noperating system newer than 25 years old to compile Git correctly. As\nI've said or implied in previous threads, I'm also fine requiring C99\n(vendors having had over two decades to implement it) and only\nsupporting OSes with vendor security support, although obviously these\nlatter two items are much more controversial.\n\nI'm fine leaving the commit message as it stands, given the brevity of\nthe patch and that in the technology field, the affected versions are\nnot in any way \"modern,\" but of course I wouldn't object to a reroll.\nIt's fine, should that happen, to include any of this email in the\ncommit message.\n\n[0] https://en.wikipedia.org/wiki/Microsoft_Visual_C%2B%2B\n[1] https://en.wikipedia.org/wiki/Macintosh\n-- \nbrian m. carlson (he/him or they/them)\nHouston, Texas, US\n"},{"id":"409466","messageId":"xmqqft5h92fm.fsf@gitster.c.googlers.com","threadId":"54598","inReplyTo":"20201110023620.GH6252@camp.crustytoothpaste.net","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T06:00:13Z","receivedAt":"2020-11-10T06:00:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On 2020-11-10 at 00:31:27, Jeff King wrote:\n>> On Sun, Nov 08, 2020 at 11:57:41AM +0200, Daniel Gurney wrote:\n>> \n>> > Modern MSVC or Windows versions don't support big-endian, so it's\n>> > unnecessary to consider architectures when using it.\n>> \n>> This made me wonder if we support any non-modern versions (which would\n>> be negatively impacted).\n>\n> I'm pretty sure we don't. As I said, we're using several C99 features\n> and that version precedes the C99 standard (and 1999).\n>\n>> From the earlier thread at [1], it sounds like probably not, but I\n>> wonder if we can offer a stronger argument there (or just define\n>> \"modern\" a little more clearly).\n>\n> According to Wikipedia[0]:\n>\n> Visual C++ 2013 [12.0] finally added support for various C99 features\n> in its C mode (including designated initializers, compound literals,\n> and the _Bool type), though it was still not complete. Visual C++ 2015\n> further improved the C99 support, with full support of the C99\n> Standard Library, except for features that require C99 language\n> features not yet supported by the compiler.\n>\n> The version mentioned that supported MIPS, Alpha, and m68k was Visual\n> C++ 2.0 RISC Edition. While Wikipedia doesn't mention its release date,\n> its successor, Visual C++ 4.0, was released in 1995. The m68k version\n> ran on Macs using those processors, and Apple abandoned m68k for PowerPC\n> in 1994[1].\n\nSo, \n\n\tThe only versions of MSVC that support big-endian are too\n\tancient and do not understand some C99 features we use in\n\tour codebase, so it is unnecessary...\n\nwould be sufficient?\n\n> I'm entirely comfortable with requiring that people use a compiler and\n> operating system newer than 25 years old to compile Git correctly. As\n> I've said or implied in previous threads, I'm also fine requiring C99\n> (vendors having had over two decades to implement it) and only\n> supporting OSes with vendor security support, although obviously these\n> latter two items are much more controversial.\n\nMaybe controversial, but worth at least laying the ground rules\nahead of time?\n\nDo we have any specific feature we avoid only due to portability\nconcerns? Declaring an identifier in the first part of for() is the\nonly thing that comes to my mind, but there may be others. I think\nwe should consider how well each individual feature is supported by\nsystems we care about as we feel the need.\n\nThanks.\n"},{"id":"409487","messageId":"nycvar.QRO.7.76.6.2011101500370.18437@tvgsbejvaqbjf.bet","threadId":"54598","inReplyTo":"xmqqft5h92fm.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-11-10T14:04:44Z","receivedAt":"2020-11-10T14:05:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 9 Nov 2020, Junio C Hamano wrote:\n\n> \"brian m. carlson\" writes:\n>\n> > On 2020-11-10 at 00:31:27, Jeff King wrote:\n> >> On Sun, Nov 08, 2020 at 11:57:41AM +0200, Daniel Gurney wrote:\n> >>\n> >> > Modern MSVC or Windows versions don't support big-endian, so it's\n> >> > unnecessary to consider architectures when using it.\n> >>\n> >> This made me wonder if we support any non-modern versions (which would\n> >> be negatively impacted).\n> >\n> > I'm pretty sure we don't. As I said, we're using several C99 features\n> > and that version precedes the C99 standard (and 1999).\n> >\n> >> From the earlier thread at [1], it sounds like probably not, but I\n> >> wonder if we can offer a stronger argument there (or just define\n> >> \"modern\" a little more clearly).\n> >\n> > According to Wikipedia[0]:\n> >\n> > Visual C++ 2013 [12.0] finally added support for various C99 features\n> > in its C mode (including designated initializers, compound literals,\n> > and the _Bool type), though it was still not complete. Visual C++ 2015\n> > further improved the C99 support, with full support of the C99\n> > Standard Library, except for features that require C99 language\n> > features not yet supported by the compiler.\n> >\n> > The version mentioned that supported MIPS, Alpha, and m68k was Visual\n> > C++ 2.0 RISC Edition. While Wikipedia doesn't mention its release date,\n> > its successor, Visual C++ 4.0, was released in 1995. The m68k version\n> > ran on Macs using those processors, and Apple abandoned m68k for PowerPC\n> > in 1994[1].\n>\n> So,\n>\n> \tThe only versions of MSVC that support big-endian are too\n> \tancient and do not understand some C99 features we use in\n> \tour codebase, so it is unnecessary...\n>\n> would be sufficient?\n>\n> > I'm entirely comfortable with requiring that people use a compiler and\n> > operating system newer than 25 years old to compile Git correctly. As\n> > I've said or implied in previous threads, I'm also fine requiring C99\n> > (vendors having had over two decades to implement it) and only\n> > supporting OSes with vendor security support, although obviously these\n> > latter two items are much more controversial.\n>\n> Maybe controversial, but worth at least laying the ground rules\n> ahead of time?\n>\n> Do we have any specific feature we avoid only due to portability\n> concerns? Declaring an identifier in the first part of for() is the\n> only thing that comes to my mind, but there may be others. I think\n> we should consider how well each individual feature is supported by\n> systems we care about as we feel the need.\n\nI would feel a bit more comfortable reintroducing the part that\nspecifically checks for x86, x86_64 and ARM64, for the reasons I outlined\nin my reply to a previous version of this patch: just because the MSVC\nversions we can currently use to build Git currently only supports little\nendian does not mean that all future versions will do. Point in reference:\nyou can build Linux applications in Visual Studio like _right now_ [*1*].\n\nCiao,\nDscho\n\nFootnote *1*: It currently uses GCC, but who says it always will?\nhttps://docs.microsoft.com/en-us/cpp/linux/cmake-linux-project\n"},{"id":"409490","messageId":"20201110142122.GA1311261@coredump.intra.peff.net","threadId":"54598","inReplyTo":"20201110023620.GH6252@camp.crustytoothpaste.net","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-11-10T14:21:22Z","receivedAt":"2020-11-10T14:21:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 10, 2020 at 02:36:20AM +0000, brian m. carlson wrote:\n\n> > > Modern MSVC or Windows versions don't support big-endian, so it's\n> > > unnecessary to consider architectures when using it.\n> > \n> > This made me wonder if we support any non-modern versions (which would\n> > be negatively impacted).\n> \n> I'm pretty sure we don't. As I said, we're using several C99 features\n> and that version precedes the C99 standard (and 1999).\n\nYour response was much more thorough than I anticipated. What I was\nreally going for in the commit message was just laying out \"modern\" a\nbit more clearly, like:\n\n No version of MSVC or Windows has supported a big-endian platform\n since the mid-90's. Git wouldn't build with these pre-C99 compilers\n these days anyway, so we can assume that MSVC is always little-endian.\n\n> I'm fine leaving the commit message as it stands, given the brevity of\n> the patch and that in the technology field, the affected versions are\n> not in any way \"modern,\" but of course I wouldn't object to a reroll.\n> It's fine, should that happen, to include any of this email in the\n> commit message.\n\nI often find that the briefer the patch, the more I need to revisit the\nassumptions of the author (myself included!). :)\n\nBut I am also OK either way; when commit messages are insufficient, I\noften go back to the list archive to get more details, and now it's well\ndocumented here.\n\n-Peff\n"},{"id":"409493","messageId":"f41271c0-973d-ee3f-9535-d7fbc5b073cf@gmail.com","threadId":"54598","inReplyTo":"nycvar.QRO.7.76.6.2011101500370.18437@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Daniel Gurney","fromEmail":"dgurney99@gmail.com","sentAt":"2020-11-10T15:35:08Z","receivedAt":"2020-11-10T15:35:15Z","isPatch":true,"sender":{"key":"dgurney99@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12816807?v=4"},"body":"Hello,\n\nOn 10/11/2020 16:04, Johannes Schindelin wrote:\n > Point in reference:\n > you can build Linux applications in Visual Studio like _right now_ [*1*].\n >\n > Ciao,\n > Dscho\n >\n > Footnote *1*: It currently uses GCC, but who says it always will?\n > https://docs.microsoft.com/en-us/cpp/linux/cmake-linux-project\n\nWhen it comes to building C++ code MSVC uses Microsoft's C++ Standard \nLibrary, and one of its stated non-goals[1] is being ported to other \nsystems. I assume the same applies for their C library. Therefore to me \na scenario where a future version of MSVC would build non-Windows code, \nlet alone for a big-endian architecture, seems extremely unlikely.\n\nThat said, I understand your overall point of view perfectly, and I'm \nfine with either one of my patches being applied since the end result \ntoday is the same.\n\n- Daniel\n\n[1] https://github.com/microsoft/STL#non-goals\n\n\n"},{"id":"409495","messageId":"nycvar.QRO.7.76.6.2011101643460.18437@tvgsbejvaqbjf.bet","threadId":"54598","inReplyTo":"f41271c0-973d-ee3f-9535-d7fbc5b073cf@gmail.com","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-11-10T15:47:17Z","receivedAt":"2020-11-10T15:47:39Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Daniel,\n\nOn Tue, 10 Nov 2020, Daniel Gurney wrote:\n\n> On 10/11/2020 16:04, Johannes Schindelin wrote:\n> > Point in reference:\n> > you can build Linux applications in Visual Studio like _right now_ [*1*].\n> >\n> > Ciao,\n> > Dscho\n> >\n> > Footnote *1*: It currently uses GCC, but who says it always will?\n> > https://docs.microsoft.com/en-us/cpp/linux/cmake-linux-project\n>\n> When it comes to building C++ code MSVC uses Microsoft's C++ Standard Library,\n> and one of its stated non-goals[1] is being ported to other systems. I assume\n> the same applies for their C library. Therefore to me a scenario where a\n> future version of MSVC would build non-Windows code, let alone for a\n> big-endian architecture, seems extremely unlikely.\n\nWell, we know for a fact that at least XBox is a big-endian system and\nther are rumors out there that Visual Studio _might_ be used to build\nsoftware for that system.\n\nOf course, I am not suggesting that we should build Git for XBox. But it\nseems dangerous to me to make too many assumptions.\n\n> That said, I understand your overall point of view perfectly, and I'm fine\n> with either one of my patches being applied since the end result today is the\n> same.\n\nAs a maintainer, I am less concerned about the \"result today\" than I am\nabout keeping things easy and effortless to maintain. One of your patches\naccomplishes that. The other one made it into `next`:\nhttps://github.com/git/git/commit/91a67b86f77\n\nCiao,\nDscho\n"},{"id":"409502","messageId":"xmqqy2j96sd4.fsf@gitster.c.googlers.com","threadId":"54598","inReplyTo":"nycvar.QRO.7.76.6.2011101643460.18437@tvgsbejvaqbjf.bet","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T17:20:39Z","receivedAt":"2020-11-10T17:20:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> As a maintainer, I am less concerned about the \"result today\" than I am\n> about keeping things easy and effortless to maintain. One of your patches\n> accomplishes that. The other one made it into `next`:\n> https://github.com/git/git/commit/91a67b86f77\n\nI do not think reverting it and requeuing\n\nhttps://lore.kernel.org/git/20201107221916.1428757-1-dgurney99@gmail.com/\n\nwould help future folks why we ignore _MSC_VER as any sign usable to\ndetect endianness, so I'd prefer to see a patch *on top* of 1af265f0\n(compat/bswap.h: simplify MSVC endianness detection, 2020-11-08),\nwhich is 91a67b86f77^2, that explains why we prefer to list archs\nexplicitly in its log message, which would be the primary value of\nthat commit.\n\nSomething along this line, perhaps?\n\n-- >8 --\n\nSubject: compat/bswap.h: do not assume MSVC is little-endian only\n\nEarlier, with 1af265f0 (compat/bswap.h: simplify MSVC endianness\ndetection, 2020-11-08), we tried to simplify endianness detection\nused in compat/bswap.h by assuming that any version Git compiled by\nMSVC (detected by _MSC_VER preprocessor macro) is meant to run on\nlittle endian boxes, as the versions of old MSVC that support m68k\nand MIPS do not support some C99 features used in the codebase\nanyway.\n\nWhile it might hold true that modern versions of Windows are all\nlittle-endian, MSVC is and/or can be ported to build for big-endian\nboxes, so tying _MSC_VER with endianness is a bit too restrictive.\n\nLet's go back to the old way to use _MSC_VER to learn what\npreprocessor macros compiler uses to tell us which arch we are\nbuilding for, and list these arches that are little-endian\nexplicitly.\n\n... signed-off-by from you and helped-by from others ...\nSigned-off-by: Junio C Hamano \n---\n diffstat\n patch\n\n"},{"id":"409503","messageId":"xmqqtutx6rwc.fsf@gitster.c.googlers.com","threadId":"54598","inReplyTo":"xmqqy2j96sd4.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-11-10T17:30:43Z","receivedAt":"2020-11-10T17:30:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Johannes Schindelin writes:\n>\n>> As a maintainer, I am less concerned about the \"result today\" than I am\n>> about keeping things easy and effortless to maintain. One of your patches\n>> accomplishes that. The other one made it into `next`:\n>> https://github.com/git/git/commit/91a67b86f77\n>\n> I do not think reverting it and requeuing\n>\n> https://lore.kernel.org/git/20201107221916.1428757-1-dgurney99@gmail.com/\n>\n> would help future folks why we ignore _MSC_VER as any sign usable to\n> detect endianness, so I'd prefer to see a patch *on top* of 1af265f0\n> (compat/bswap.h: simplify MSVC endianness detection, 2020-11-08),\n> which is 91a67b86f77^2, that explains why we prefer to list archs\n> explicitly in its log message, which would be the primary value of\n> that commit.\n>\n> Something along this line, perhaps?\n>\n> -- >8 --\n>\n> Subject: compat/bswap.h: do not assume MSVC is little-endian only\n>\n> Earlier, with 1af265f0 (compat/bswap.h: simplify MSVC endianness\n> detection, 2020-11-08), we tried to simplify endianness detection\n> used in compat/bswap.h by assuming that any version Git compiled by\n> MSVC (detected by _MSC_VER preprocessor macro) is meant to run on\n> little endian boxes, as the versions of old MSVC that support m68k\n> and MIPS do not support some C99 features used in the codebase\n> anyway.\n>\n> While it might hold true that modern versions of Windows are all\n> little-endian, MSVC is and/or can be ported to build for big-endian\n> boxes, so tying _MSC_VER with endianness is a bit too restrictive.\n>\n> Let's go back to the old way to use _MSC_VER to learn what\n> preprocessor macros compiler uses to tell us which arch we are\n> building for, and list these arches that are little-endian\n> explicitly.\n>\n> ... signed-off-by from you and helped-by from others ...\n> Signed-off-by: Junio C Hamano \n> ---\n> diffstat\n> patch\n\nDaniel's patch adds _M_ARM64 to the list, but do we need to do\nanything further to tell the endian on such a bi-endian arch, or\ndoes MSVC only support little-endian for that architecture? \n\nJust double-checking as the \"confusion\" that started this thread\ncame from an assumption that MSVC == Windows == big-endian, and you\ntold us MSVC != Windows. Now the patch assumes ARM64-on-MSVC is\nlittle-endian only and we want to make sure that assumption is true.\n\nAnd perhaps it is worth documenting in the log, perhaps\n\n\t... that are little-endian explicitly. Note that ARM64 is\n\tbi-endian in nature but we treat it little-endian as MSVC\n\tdoes not treat the arch as bi-endian.\n\nor something like that at the end (I do not know what MSVC actually\ndoes---just illustrating the level of details I expect in the\nexplanation).\n\nThanks.\n"},{"id":"409561","messageId":"nycvar.QRO.7.76.6.2011102333530.18437@tvgsbejvaqbjf.bet","threadId":"54598","inReplyTo":"xmqqtutx6rwc.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v2] compat/bswap.h: simplify MSVC endianness detection","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-11-10T22:37:33Z","receivedAt":"2020-11-10T22:37:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 10 Nov 2020, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n>\n> > Johannes Schindelin writes:\n> >\n> >> As a maintainer, I am less concerned about the \"result today\" than I am\n> >> about keeping things easy and effortless to maintain. One of your patches\n> >> accomplishes that. The other one made it into `next`:\n> >> https://github.com/git/git/commit/91a67b86f77\n> >\n> > I do not think reverting it and requeuing\n> >\n> > https://lore.kernel.org/git/20201107221916.1428757-1-dgurney99@gmail.com/\n> >\n> > would help future folks why we ignore _MSC_VER as any sign usable to\n> > detect endianness, so I'd prefer to see a patch *on top* of 1af265f0\n> > (compat/bswap.h: simplify MSVC endianness detection, 2020-11-08),\n> > which is 91a67b86f77^2, that explains why we prefer to list archs\n> > explicitly in its log message, which would be the primary value of\n> > that commit.\n> >\n> > Something along this line, perhaps?\n> >\n> > -- >8 --\n> >\n> > Subject: compat/bswap.h: do not assume MSVC is little-endian only\n> >\n> > Earlier, with 1af265f0 (compat/bswap.h: simplify MSVC endianness\n> > detection, 2020-11-08), we tried to simplify endianness detection\n> > used in compat/bswap.h by assuming that any version Git compiled by\n> > MSVC (detected by _MSC_VER preprocessor macro) is meant to run on\n> > little endian boxes, as the versions of old MSVC that support m68k\n> > and MIPS do not support some C99 features used in the codebase\n> > anyway.\n> >\n> > While it might hold true that modern versions of Windows are all\n> > little-endian, MSVC is and/or can be ported to build for big-endian\n> > boxes, so tying _MSC_VER with endianness is a bit too restrictive.\n> >\n> > Let's go back to the old way to use _MSC_VER to learn what\n> > preprocessor macros compiler uses to tell us which arch we are\n> > building for, and list these arches that are little-endian\n> > explicitly.\n> >\n> > ... signed-off-by from you and helped-by from others ...\n> > Signed-off-by: Junio C Hamano \n> > ---\n> > diffstat\n> > patch\n>\n> Daniel's patch adds _M_ARM64 to the list, but do we need to do\n> anything further to tell the endian on such a bi-endian arch, or\n> does MSVC only support little-endian for that architecture?\n\nYes, MSVC only supports little-endian for that architecture. From\nhttps://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions?view=msvc-160#endianness:\n\n\tEndianness\n\n\tAs with the ARM32 version of Windows, on ARM64 Windows executes in\n\tlittle-endian mode. Switching endianness is difficult to achieve\n\twithout kernel mode support in AArch64, so it's easier to enforce.\n\n> Just double-checking as the \"confusion\" that started this thread\n> came from an assumption that MSVC == Windows == big-endian, and you\n> told us MSVC != Windows. Now the patch assumes ARM64-on-MSVC is\n> little-endian only and we want to make sure that assumption is true.\n\nI did not say that MSVC != Windows, but Visual Studio != Windows. But I\ndid say that I do not want to assume MSVC == Windows for all eternity.\n\n> And perhaps it is worth documenting in the log, perhaps\n>\n> \t... that are little-endian explicitly. Note that ARM64 is\n> \tbi-endian in nature but we treat it little-endian as MSVC\n> \tdoes not treat the arch as bi-endian.\n>\n> or something like that at the end (I do not know what MSVC actually\n> does---just illustrating the level of details I expect in the\n> explanation).\n\nAbsolutely. If nobody beats me to the punch, I hope to get around to\nprepare the patch that you suggested.\n\nThanks,\nDscho\n"}]}