Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added __amigaos3__ define to standard set of default defines. #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jens-maus
Copy link
Member

added amigaos3 define to standard set of default defines. This should allow to detect classic Amiga systems (<= AmigaOS3) in multi-platform projects more easily. This refs adtools/amigaos-cross-toolchain#62.

…ould allow to detect classic Amiga systems (<= AmigaOS3) in multi-platform projects more easily. This refs adtools/amigaos-cross-toolchain#62.
@sezero
Copy link

sezero commented Jan 10, 2017

This will be a non-standart macro. There are other ways of detecting aos3, e.g.:
https://sourceforge.net/p/uhexen2/code/HEAD/tree/trunk/common/arch_def.h
(search PLATFORM_AMIGAOS and PLATFORM_AMIGAOS3 in there. No, it is
not as easy as checking __amigaos3__ but it relies on standart macros.)

@jens-maus
Copy link
Member Author

jens-maus commented Jan 10, 2017

As stated in the original issue ticket (adtools/amigaos-cross-toolchain#62) testing for non-amigaos4, non-morphos and non-aros is of course also possible and of course using the header file you are proposing is also possible. However, for the sake of simplicity adding a new __amigaos3__ define would be even more simple and straight forward. In fact, that's exactly what we are currently doing for all our multi-platform projects (we are always adding -D__amigaos3__) to the cross-compiler execution when using the OS3 cross compiler. And sorry, but I don't understand what you mean with "non-standard macro"? Aren't we maintaining the GCC sources here to actually set new standards and improve the way of using GCC for AmigaOS projects? That's why I feel we should also go and define new ways of using GCC with Amiga projects. AmigaOS4 also has it's __amigaos4__, MorphOS has it's __MORPHOS__ and AROS has it's __AROS__ standard define. Thus, classic cross compilers should IMHO get something like __amigaos3__ to make it easier for developers and not having to rely on external header files or workaround like we added. In addition, I can't see any negative effects by adding this additional standard define. Existing projects would anyway not use this new macro and new projects can easily then use it as soon as they use the new cross compiler toolchain we are developing here.

@sezero
Copy link

sezero commented Jan 10, 2017

Aren't we maintaining the GCC sources here to actually set new standards
[...]
AmigaOS4 also has it's amigaos4, MorphOS has it's MORPHOS

For that you are right. My argument is that if someone naively relies on the new
macro and then uses an older compiler, he'll get an unpleasant surprise.

@jens-maus
Copy link
Member Author

Well, as usual people can always shoot themselves if they don't know what they are doing. So, if a developer is actively adding the __amigaos3__ define at some point they should already know that this is a new macro that was added at a certain point (or otherwise they don't know about it anyway). So then if they revert to an older GCC version and get errors during compilation they will remember that they have added the use of this new define at some point and revert it again. And of course future moves forward and so should we. IMHO we are here to improve something or otherwise this project doesn't make any sense and we should keep our Amiga patches for GCC 2.9.53 in a state like it was initially developed including bugs/workaround, etc. And thus this project would be senseless.

@sezero
Copy link

sezero commented Jan 10, 2017

OK.

@tboeckel
Copy link

AmigaOS 1.x and AmigaOS 2.x are even more outdated and dead than AmigaOS 3.x. Look forward, not backward. Personally I consider AmigaOS 3.9 as the absolute minimum to support today. If someone really wants to stick with an AmigaOS 3.0 or 3.1 system then let him. These people cannot be helped.

@cahirwpz
Copy link
Member

According to clib2's documentation it supports Kickstart 2.04 and above (can somebody conform that?). libnix with some caveats supports Kickstart 1.x. If so... it would be appropriate to add __amigaos2__ to CPP_CLIB2_SPEC and __amigaos1__ to CPP_LIBNIX_SPEC. If we want to have only one preprocessor define to rule them all I suppose __amigaos_classic__ would be better that __amigaos3__, as the latter implies a specific OS version is used. What do you think?

@tboeckel I compile programs to run on KS 1.x when I code for demoscene.

@jens-maus
Copy link
Member Author

@cahirwpz While I understand your argument I still have to disagree because using a general __amigaos3__ would be IMHO more appropriate than introducing something like __amigaos1__ and __amigaos2__ because the differentiation between OS1.x and OS2.x or OS3.x should be done on runtime IMHO. Also using something like __amigaos_classic__ seems to be way to lengthy and uncommon. As said, IMHO we have actually four classes of Amiga platforms:

  1. AmigaOS4 (PPC) – __amigaos4__
  2. MorphOS (PPC) – __MORPHOS__
  3. AROS (PPC, x86, etc.) – __AROS__
  4. AmigaOS3 (m68k) – No unique define to identify

And here, to be in line what the other platforms do using __amigaos3__ seems to be more appropriate to me even though there is a number 3 within the define. Please note that AmigaOS4 is also not introducing separate defines for AmigaOS4.1 or AmigaOS4.2 ans "amigaos4" simply refers to "this is the next-generation AmigaOS version with a partly incompatible API (interfaces, etc.) to old Amiga systems". And in line with that definition using __amigaos3__ for the whole set of "old" Amiga systems (OS1, OS2, OS3) seems to be most appropriate and compatible to the way the other Amiga platforms are defining their "platform identification defines", as I would call them. So using "3" or "4" in the defines just refers to "old generation" and "new generation" IMHO.

@cahirwpz
Copy link
Member

@jens-maus What about the idea of placing __amigaos3__ only on CPP_CLIB2_SPEC?

@jens-maus
Copy link
Member Author

While this is also, of course, possible, I just wonder why you insist so much in not making __amigaos3__ a new standard define for <= OS3 builds? I really see no big deal/problem in introducing this define for all clib variants.

@cahirwpz
Copy link
Member

This is all about development policy. For each problem I try to find a solution that:

  • is minimal,
  • has the least chance of negative side effects,
  • will not change the default behaviour,
  • will not confuse a user.

If I saw __amigaos3__ in source files it would me make think that compiled program was going to run on AmigaOS 3.x. But really it means that the program is compiled for classic (m68k) AmigaOS.

IIUC you think it's ok to introduce __amigaos3__ preprocessor define if -mcrt=clib2* options is passed to the compiler?

@jens-maus
Copy link
Member Author

Well, if you insist on introducing __amigaos3__ only for CLIB2 compilations I am also fine with that because we/I only use clib2 anyway nowadays due to the fact that libnix is so utterly old and unmaintained :)

However, I do still believe that adding it for all clib variants would be most favorable and uncritical in the end. As said, if a developer proactively adds __amigaos3__ he must know what this means. And now that you will introduce __amigaos3__ for clib2 only you will anyway sooner or later stumble over sources with this new define. What I am more worried about is that by binding this define to clib2 only you will then again introduce something that will end up user having to create complicated #ifdef statements if they want to check for classic system compilation in general. because in theory then __amigaos3__ is just a replacement for #ifdef __CLIB2__ && __mc68000__ and nothing more and that was not the initial idea of my pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants