psmedley / gcc

GNU General Public License v2.0
7 stars 1 forks source link

cstddef declares max_align_t, which is missing in kLIBC stddef.h, in std namespace #12

Closed komh closed 8 years ago

komh commented 9 years ago

gcc492 is missing a patch for cstddef which is in gcc491. kLIBC does not support max_align_t.

And sorry about that this is not for a git repo, but a release.

diff -uNr  include/c++/4.9.2/cstddef.org include/c++/4.9.2/cstddef
--- include/c++/4.9.2/cstddef.org  2014-11-01 11:21:16.000000000 +0900
+++ include/c++/4.9.2/cstddef 2015-01-12 18:29:48.000000000 +0900
@@ -44,7 +44,7 @@
 #include <bits/c++config.h>
 #include <stddef.h>

-#if __cplusplus >= 201103L
+#if __cplusplus >= 201103L && !defined (__KLIBC__)
 namespace std
 {
   // We handle size_t, ptrdiff_t, and nullptr_t in c++config.h.
ydario commented 9 years ago

gcc 4.9.2 is compiled using libc patched headers in order to support max_align_t, so unless you see some other kind of problem please close this ticket.

dmik commented 9 years ago

Support for max_align_t is added to our kLIBC RPM so it isn't necessary to fix it in GCC. As far as I see, there is a kLIBC ticket by you @komh: http://trac.netlabs.org/libc/ticket/297. So let's wait until it's "officially" done by Knut and use our RPM until that.

komh commented 9 years ago

If so, Paul does not distribute his build any more ?

psmedley commented 9 years ago

KO,

I'll likely to continue to package a build for those that don't want/like rpm.

Cheers,

Paul On 3 Feb 2015 1:45 pm, "KO Myung-Hun" notifications@github.com wrote:

If so, Paul does not distribute his build any more ?

— Reply to this email directly or view it on GitHub https://github.com/psmedley/gcc/issues/12#issuecomment-72585392.

ydario commented 9 years ago

This is just a tool for sharing the source code. This way we can have more than one guy working on the software.

KO, please continue to contribute to this project, and you can use svn repository at netlabs.org for a lot of other projects. e.g. I saw you updated libtool while we worked on it too. Using a single repo will help us to get more projects done.

dmik commented 9 years ago

@psmedley btw, our RPM build infrastructure is able to automatically create ZIPs for all RPM packages we create. Our builds use the hard-coded /@unixroot prefix everywhere but since it can be set to whatever the user wants with UNIXROOT, that should be fine for most cases.

komh commented 9 years ago

@psmedley Thanks Paul. Then, would you mind applying this patch or the patches I sent you before to your local repo ?

@ydario I'm already using svn repo at netlabs.org. For example, pthread.

@dmik However, there are those including me who do not want to install all tools to the same directory. For this, /@unixroot is not proper.

dmik commented 9 years ago

@komh The fact that we prepend @unixroot to hard-coded paths doesn't prevent you from installing the zipped RPM application in any directory of your choice (or even installing each application in its own directory). It just adds an extra component to the path that may be expanded to something with UNIXROOT if you wish or used as is (as a literal directory name) if UNIXROOT is unset. Other than that, our RPM->ZIP GCC build doesn't differ from what Paul does. And I strongly dislike the idea of having two separate distributions of the very same thing as it increases entropy and confuses end users. So I advise all of us to avoid it whenever possible. This relates to your own releases of Unix tools (and to @psmedley as well :) Since our RPM builds also produce ZIPs (and your ZIP builds don't also produce RPMs) I strongly suggest to stick with our build ecosystem even for your own projects (at least for the ones we don't disagree on the patch sets).

Of course, there are programs which only work from a fixed directory tree out-of-the-box and a series of fixes is needed to make them roaming (i.e. work from any directory). Usually we reject such patches in our ports repositories but if we come to the above agreement we are ready to discuss this topic and apply such patches in cases when there is absolutely no other way to make the application roaming.

And of course if you want a different fixed directory structure than ours (like C:/mycoolunix/usr/bin), then there is no other choice than to build the application yourself — and no problems with that. But please always remember about entropy.

komh commented 9 years ago

I've patched cstddef another way. Those who not using RPM may have problems. I think, this patch is both for RPM users and non-RPM users.

From b4da813409ea5d660eb6dd11e78a991b48b8efc3 Mon Sep 17 00:00:00 2001
From: KO Myung-Hun <komh@chollian.net>
Date: Mon, 29 Jun 2015 17:28:46 +0900
Subject: [PATCH 3/3] OS/2: check _GCC_MAX_ALIGN_T before using ::max_align_t

---
 libstdc++-v3/include/c_global/cstddef | 2 +-
 libstdc++-v3/include/c_std/cstddef    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libstdc++-v3/include/c_global/cstddef b/libstdc++-v3/include/c_global/cstddef
index 1a87dd6..b4cb538 100644
--- a/libstdc++-v3/include/c_global/cstddef
+++ b/libstdc++-v3/include/c_global/cstddef
@@ -44,7 +44,7 @@
 #include <bits/c++config.h>
 #include <stddef.h>

-#if __cplusplus >= 201103L
+#if __cplusplus >= 201103L && defined(_GCC_MAX_ALIGN_T)
 namespace std
 {
   // We handle size_t, ptrdiff_t, and nullptr_t in c++config.h.
diff --git a/libstdc++-v3/include/c_std/cstddef b/libstdc++-v3/include/c_std/cstddef
index f4845f0..592b7dc 100644
--- a/libstdc++-v3/include/c_std/cstddef
+++ b/libstdc++-v3/include/c_std/cstddef
@@ -44,7 +44,7 @@
 #include <bits/c++config.h>
 #include <stddef.h>

-#if __cplusplus >= 201103L
+#if __cplusplus >= 201103L && defined(_GCC_MAX_ALIGN_T)
 namespace std
 {
   // We handle size_t, ptrdiff_t, and nullptr_t in c++config.h.
-- 
1.9.5
komh commented 9 years ago

Ooops... How can I reopen this issue ?

dmik commented 9 years ago

There is a button below, saying "Reopen and comment". But first tell me please why is the kLIBC patch not sufficient for you?

komh commented 9 years ago

Hmmm... perhaps I don't have proper rights. I can see just 'Commen't not 'Reopen and comment'.

For kLIBC patch, the important fact is that the patch is not applied to kLIBC, yet. So, non-RPM users(original kLIBC + Paul's gcc492) will encounter max_align_t errors when using cstddef of C++11.

This repo is only for RPM users ? If not, non-RPM users should be considered.

BTW, kLIBC was forked ? At least, stddef.h of RPM is different from original stddef.h

komh commented 8 years ago

Pushed as commit 13e9736b5c4f3dfa44ccd96893fee01e868fe3b6.

dmik commented 8 years ago

@komh No, kLIBC was not forked, we just patch some critical things in the .spec file (the script controlling the RPM build) because Knut is rather slow in applying our patches to his "official" tree.

komh commented 8 years ago

After all, kLIBC of RPM distribution is different from kLIBC of official distribution. Right ? Then, what is this different from 'fork' ?

dmik commented 8 years ago

It differs in that it's not a separate work, there is no principal difference or incompatibility in functionality, we don't have our own road map, we don't maintain tickets, etc. It's very common to provide patches in RPM for distribution-specific nuances that don't qualify (and neither they make any sense) as a full fork on their own.

komh commented 8 years ago

Thoughts about 'fork' seems to be different each other.

dmik commented 8 years ago

Yes, seems so. But if we take your definition of "fork", then every Fedora RPM package will become a fork (because every single one I saw there includes some patches — and I've seen a lot of them since I do OS/2 RPMs based on Fedora RPMs)...