-
Notifications
You must be signed in to change notification settings - Fork 98
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
Improved building via CMake #184
Conversation
Personally I don't know anything about CMake, so if people have nothing against this merge, I'll apply it. :) |
I am mostly worried about amiga and legacy building, since the opcode generation files has been moved to the root directory and is now used as a file that is compiled with rather than being |
1fcc65d
to
5812dee
Compare
Anything new? I just rebased and tested it (on Ubuntu). I think the cmake 'code' I've written is understandable enough. See the "files changed" link above the first post if you like to see all the changes in one diff. |
Also, the changes to WLA's main.c might make it not to compile on Amiga? |
On Sat, Sep 29, 2018 at 8:10 PM Neui ***@***.***> wrote:
Anything new? I just rebased and tested it (on Ubuntu). I think the cmake
'code' I've written is understandable enough. See the "files changed" link
<https://github.com/vhelin/wla-dx/pull/184/files> above the first post if
you like to see all the changes in one diff.
I don't know much about CMake, but the cmake stuff you've written looks
good to my eye. I'm ok with this, even though WLA might not compile on
Amiga any more, but then again is Amiga support worth keeping in 2018? :)
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGHtUtF9ZsnEVFtavV6XoHikTcljxiiiks5uf6l2gaJpZM4U5j8V>
.
|
Well not with the old build files. I've tried to create "legacy makefiles" (in an another branch), which I tried to base off some things from the makefiles from amiga (which used gcc to my surprise, so no real changes was needed), but since I don't have an amiga machine, I can't test it. Maybe cross compilation is an option. |
My personal opinion is that there are vendor tools (Deluxe Paint, anyone?) for Amiga, DOS, and friends that are still usable for old consoles and for whom support will probably never be modernized. Having access to WLA-DX old machines is still valuable IMO. That said, I don't have an Amiga and can't test the old scripts.
This is also possible, provided an Amiga 3.x compiler exists on Windows/wherever. |
On Sun, Sep 30, 2018 at 8:31 PM William D. Jones ***@***.***> wrote:
I'm ok with this, even though WLA might not compile on
Amiga any more, but then again is Amiga support worth keeping in 2018? :)
My personal opinion is that there are vendor tools (Deluxe Paint, anyone?)
for Amiga, DOS, and friends that are still usable for old consoles and for
whom support will probably never be modernized. Having access to WLA-DX old
machines is still valuable IMO.
That said, I don't have an Amiga and can't test the old scripts. cmake
will never run on Amiga or DOS (and it's not possible to run cmake-generated
build scripts on anywhere except the host).
I have never actually heard from anyone that they were using WLA DX on an
Amiga computer. The Amiga support came from the beginning of WLA DX, the
first years I coded WLA DX I still had Amiga, and used it casually now and
then, but I was moving to use Linux and PC hardware. Back then Amiga
support was just cool to have, multiple platforms with different kinds
compilers reporting different kinds of issues. Today in 2018 I'm kinda
ready to drop Amiga support... These days you can buy an okayish Windows PC
laptop for less than the Apollo 68080 accelerator for A500/600...
… Maybe cross compilation is an option.
This is also possible, provided an Amiga 3.x compiler exists on
Windows/wherever.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGHtUqGx_WFuvv41tZi58cyC3aRfUt0mks5ugP_ygaJpZM4U5j8V>
.
|
It's your codebase, feel free to drop Amiga support :). I would still like the code to remain portable when possible, however (in other words, not tied to a specific compiler). @Neui This means I still want the |
Note that existing makefiles in the historical folder would break. Also, you can't build on (Free)DOS anyway due to 8.3 name restriction, but you probably can cross-compile to it. In the case of amiga, there is a special table marked as What I am unsure is if missing this |
Perhaps, before merging this pull request, we should release 9.8? Make this
the point where we finally (possibly) dropped Amiga support? 9.8 feature
list is about half the size of 9.7 feature list, but support breaking could
be enough for a new version?
On Sun, Sep 30, 2018 at 9:17 PM Neui ***@***.***> wrote:
Note that existing makefiles in the historical folder would break. Also,
you can't build on (Free)DOS anyway due to 8.3 name restriction, but you
probably can cross-compile to it.
In the case of amiga, there is a special table marked as __far followed
by a table that is being #include d. The change is that this __far has
been commented out in favour of just saying there is an table with that
variable name (extern int opcode_n[256];) and the actual table is
compiled separately and linked together by the linker. (Think like doing gcc
main.c opcode_table.c rather than just gcc main.c with main.c including
opcode_table.c) I've done this to make the source file less depended on
those big #if #elif chains.
I think that that should still work, but does the string joining you have
now in main.c work with the ancient Amiga GCC compiler?
… What I am unsure is if missing this __far would break support for it.
Testing would say it, but, as I said, have no machine to test. Since it
seems you can use gcc on amiga (from what I've seen in the historical
makefiles for amiga), you probably can use a generated makefile to test.
(I've a branch legacy-mkfiles where I tried to create a makefile generator
that generates one makefile for a configuration without creating many files
for e. g. each system)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGHtUipBFEROgTQukCmeyS7Cy_TZnSSBks5ugQq-gaJpZM4U5j8V>
.
|
C89 mandates that string concatenation works, so even an ancient GCC almost certainly supports it. |
On Sun, Sep 30, 2018 at 9:49 PM William D. Jones ***@***.***> wrote:
I think that that should still work, but does the string joining you have
now in main.c work with the ancient Amiga GCC compiler?
C89 mandates that string concatenation works, so even an ancient GCC
almost certainly supports it.
Well that is good news. Does this mean that this pull request could work on
Amiga GCC? Man I wish I had the energy to setup WinUAE with gcc etc...
… —
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGHtUnC2Ig9cCZY3FUwMNL_fOKEtvcR_ks5ugRJMgaJpZM4U5j8V>
.
|
If you have a cross compiler for it as well as the system libraries, I don't see why not :). Of course that would be a lot of work, but in principle it can be done. Amiga 3.x support would be gone I assume though (using smake and friends). |
So.. what now?
Sorry, I didn't saw the edit until now. From the GCC documentation, If you want, I can set it explicitly to c89 for gcc. Edit: I found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=20631, where it seems Edit2: In CMakes code, it seems it automatically adjusts for that, so I going to change it to |
b0bd04e
to
574f0af
Compare
4bf67ae
to
23febfc
Compare
Rebased on current master so it can be merged without any conflicts. Note that the compatibility for old makefiles already has been broken by at least since PR #202 since it introduced a new file you would need to compile ( I've also forced CMake to use |
FYI, I just updated the historical makefiles to compile crc32.c.
…On Wed, Oct 31, 2018 at 4:22 PM Neui ***@***.***> wrote:
Rebased on current master so it can be merged without any conflicts. Note
that the compatibility for old makefiles already has been broken by at
least since PR #202 <#202> since it
introduced a new file you would need to compile (crc32.c).
I've also forced CMake to use -ansi now rather than -std=c89, just in
case.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AGHtUnrUgGgfx9vC23sG68bgxkHtBWGiks5uqbITgaJpZM4U5j8V>
.
|
Tyvm Ville :). Right now I've been doing time-sensitive work, but soon I'll set up an Amiga emulator of some sorts and see if the historical Makefiles still work. Ya know, just for fun :P. |
What's the status of this pull request? I know nothing about CMake so I cannot evaluate what has been done... @cr1901, can you review this? Anyone else? This pull request has been here for quite a while and I'd like to merge it soon if possible... |
As I've written before:
Maybe you could try to read them, it's a bit like a programming/scripting language. Basically this PR cleans up some stuff and uses a loop in order to build each flavor. The biggest change is that rather than |
I went through the changes, couldn't understand all the CMake stuff, but at least I did understand the changes in C code. :) Here are some notes (looking at this: https://github.com/vhelin/wla-dx/pull/184/files)
sc define AMIGA=1 define MC6801=1 main.c
That's it, I hope I caught all the bugs... |
Thank you for reviewing!
C90 and C89 are the same. CMake only just accepts
Good catch. Not sure where the description is exactly used, I'll have to try building it and look where it is being displayed. The documentation doesn't really say anything about it. It's more of an convenient packager that musn't be used. Also, it possibly can be automated (aka generate the "GB-Z80/Z80/6502/65C02/6510/65816/HUC6280/SPC-700" string using the list at the beginning of the CMakeLists, but not for now)
Good find. Why is there so much copy-paste... Also, I noticed that I don't link the (for example)
Ok, I removed it.
Thank you! Did you test it on an amiga system? By the way, on the page you looked the changes at also has a "Review Changes" button, where you can Approve/Request changes etc. Never used it, but you could look into it. Also, when hovering over a line, there is a + symbol where you can add a comment to the specified line. |
Ok!
Ah, ok, no need to change that, my bad.
Not yet. When I did the review I wasn't at home, could just read through the changes... Thanks for the fixes! I'll try to test run this on an Amiga later this week and will report back the results.
Oh, that's cool! I'll try to use those features in the future. :) |
* Some files referenced wrong files because of copy-pasting mistakes * The opcodes_x.c is now also linked with the opcode table generator Note that this assumes the object file is generated in the current directory. * Removed __far in pass_1.c because it isn't needed anymore
Encountered some errors while testing this on an Amiga:
... ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; copy makefiles/scoptions.000 to scoptions copy makefiles/scoptions.000 to opcode_table_generator/scoptions sc define AMIGA=1 define GB=1 main.c sc define AMIGA=1 define Z80=1 main.c sc define AMIGA=1 define MCS6502=1 main.c sc define AMIGA=1 define WDC65C02=1 main.c sc define AMIGA=1 define MC6800=1 main.c sc define AMIGA=1 define MC6801=1 main.c sc define AMIGA=1 define MC6809=1 main.c sc define AMIGA=1 define I8008=1 main.c sc define AMIGA=1 define MCS6510=1 main.c sc define AMIGA=1 define W65816=1 main.c sc define AMIGA=1 define SPC700=1 main.c sc define AMIGA=1 define HUC6280=1 main.c cd / ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ...
-> After these fixes it should work out-of-the-box on an Amiga. |
@vhelin When this PR is finally merged, I can look into trying to get CI builds on Amiga. It will clearly require an emulator that's command-line/headless friendly, and someone access to Amiga ROMs ($$), so Idk how feasible it is. But maybe worth a try? |
I bet that if you use the latest WinUAE it'll work just fine. It's a great emulator! :) |
* "rewrite" of the beginning of historical/amiga by vhelin * Use CFLAGS in smake.* instead of WLAFLAGS
Ok, I replaced it. Thank you!
I also did it, but there weren't any About the amiga building, putting it on an "online" instance might be problematic, so maybe keep a private machine. However then there is still the problem of automation. (Maybe some sort of serial connection?) |
Now it works on an Amiga! SAS/C complains about int opcode_n[] being defined both in pass_1.c and corresponding "opcodes_*_tables.c", but I don't understand how that is possible now that "opcodes*__tables.c" is not included in pass_1.c any more. Any ideas about that? Anyway, SAS/C survives the double declaration and can compile and link the executables. ??? |
I see that on line 126 in |
Haha, oh yes. :) I was just looking for #include's... I'm pretty sure that's it! I'll try to remove the declarations when I get home later today evening and see if the (non-fatal) error goes away. |
Ah, sorry about that. I just had too many files open in Notepad++ and perhaps looked inside wrong tabs...
I think setting this one up would take quite a lot of time and might not be really that useful in the long run. At least I don't have the space on my desk to keep an Amiga permanently there next to my monitor... Setting up WinUAE emulation might be a better solution as it doesn't take any space on the table and can be configured to be pretty fast. :) I mean I can deal with the extra work required to manually move files from my Windows PC to an Amiga once in a while when I want to test if WLA DX still compiles on it. Automating it would save something like 5-10 minutes per run, but I'm happier if I can store the Amiga away in the closet after my tests... |
My issue with doing this myself is AFAIK the Amiga compilers and the OS (Kickstart ROMs) actually cost money to get, and I have neither (money or the images) :P. |
No problem.
Well I don't mean an separate amiga machine but having a "modern" machine (or even your current one or one on the web) that does it (via emulation), and the serial was just an idea to communicate "easily" between the host and the amiga to transfer data and results (if possible, I don't really know). |
Yep, you were right!
-> Compiles and links on Amiga without errors. Nice! |
Previously, there were an extern and an non-extern declarations. While it seems that gcc and clang doesn't care, SAS/C for amiga did rightfully, and thus this is now fixed.
@vhelin My subtext here is I want you to provide me with Kickstart ROMs, the Amiga OS, and the compilers if I set up Amiga CI, because I can't afford to buy them :P. |
I recommend that we'll forget that idea about CI and Amiga. :D It's not really worth it, and I can do the necessary builds (and tests) once in a while, when we feel like we need them. In another words: I guess not that often... The next good time to do it might be just before releasing WLA DX v9.10... |
Thanks for the hard work! 👍 |
This does somewhat a lot of stuff, mostly taken from #164:
CMakeLists.txt
screate_tables.bat
also has been removed as a side effect. @cr1901 is probably going to complain about this.extern
variable. I am not so sure if this is going to work on amiga (because of the_far
thing).NDEBUG
now also controls the debugging defines for wla and wlalink sinceNDEBUG
is basically in the C standard (forassert
)Please review (and test) before merging. Complains allowed.