SDL-Hercules-390 / hyperion

The SDL Hercules 4.x Hyperion version of the System/370, ESA/390, and z/Architecture Emulator
Other
246 stars 92 forks source link

Port TCPNJE support from spinhawk to SDL Hyperion #280

Closed ivan-w closed 4 years ago

ivan-w commented 4 years ago

Port Peter Coghlan's spinhawk TCPNJE 1.0 version from spinhawk to the current SDL version

PeterCoghlan commented 4 years ago

This is coming as a surprise to me. Last month, I was copied on an email which suggested I was in discussions about this happening which was also news to me.

My original intention was to provide a common codebase for tcpnje which would be compatible with and usable with all Hercules variants and which would work on the widest possible range of host platforms without changes and if I had been successful, no porting would have been required, it would just work.

Unfortunately, I have been unable to keep up with incompatibilities and changes detrimental to portability which have been introduced in SDL Hyperion and therefore having common code which works across all Hercules variants has become an impossibility for me.

I have been unable to build SDL Hyperion since sometime around 2018. I am not in a position to provide support for tcpnje in SDL Hyperion. I would like all indications that I might provide support for this arrangement removed from any code and documentation included with SDL Hyperion.

Fish-Git commented 4 years ago

@PeterCoghlan

Peter,

I cannot remember exactly when it was nor exactly what it was, but I do specifically remember you mentioning an incompatibility between SDL Hyperion and the other Herculeses (*) out there at some point in the recent past (last year or two?), which I also specifically remember having fixed for you!

What incompatibilities are you now talking about? If there are incompatibilities in SDL Hyperion that is preventing your tcpnje driver from building or working correctly with SDL Hyperion, you should create a GitHub Issue for it/them so that we (myself and the rest of the SDL Hyperion team) can work on trying to get them fixed for you!

Thanks.


(*) How the heck do you pluralize "Hercules"?

Fish-Git commented 4 years ago

I would like all indications that I might provide support for this arrangement removed from any code and documentation included with SDL Hyperion.

I am not aware that there is any. But if you can find any I'd be happy to remove it. Just point it out and it'll be gone.

PeterCoghlan commented 4 years ago

Fish,

You fixed what feels like dozens of incompatibilities and portability issues for me dating back to well before SDL Hyperion came to exist. The problem for me is these issues continue to arise quicker than I can spot them, report them and ask for them to be fixed. I don't want to be on that treadmill.

The biggest issue for me is that support for static linked executables was removed from SDL Hyperion source (in 2018 I think). This made it practically impossible to build SDL Hyperion on any platform which is neither Windows nor one of a limited range of Unix variants. Since then, as far as I can see, it has only been possible to use one of two highly platform specific methods of generating dynamically linked executables, a Windows specific method and a very non-portable Unix specific method coded in ltdl.c / hdl.c. Neither of these methods of building dynamically linked executables can be used in my environment without major surgery which would be all the more difficult due to the almost complete lack of descriptive comments in ltdl.c in particular. (I tried and gave up.) It also makes it so much more difficult to share any common code between SDL Hyperion and any other Hercules variants.

After my experience last year asking for a tiny incompatibility to be fixed (which you did fix - thanks) I felt it would be a mammoth task to ask for the restoration of support for statically linked executables [1] and even if this happened, being unable to build SDL Hyperion for so long now, I would not be aware of what new difficulties may have arisen for me since the last time I could build it, which from previous experience could be quite a few.

[1] I only made use of the support for statically linked executables present in the actual C source code which I think amounted to a few hundred lines of preprocessor macros and suchlike across all the codebase. (It was probably hard work to remove them and is likely nigh on impossible to replace them all correctly now.) I never used any of the supplied build procedures, makefiles, configure scripts, shell scripts, cmd/bat files, libtools or anything else like that which are all rumoured to not be able to properly generate statically linked executables anyway.

PeterCoghlan commented 4 years ago

I would like all indications that I might provide support for this arrangement removed from any code and documentation included with SDL Hyperion.

I am not aware that there is any. But if you can find any I'd be happy to remove it. Just point it out and it'll be gone.

There are (probably) not any statements involving me in the current SDL Hyperion codebase that I would object to but there will be if the proposal in the title of this thread goes ahead.

ivan-w commented 4 years ago

One of the thing I ran into is that there is a mixture of the use of "TCPNJE" as if it where a "typedef" and "struct TCPNJE". Possibly some compilers self correct this, but gcc doesn't. Once I fixed that, there was only a couple of compilation errors that were easy to fix (mostly the missing definition of the variable "filename" which is implicitly used in a convenience macro.

So now it compiles, but I haven't tried using it yet.

ivan-w commented 4 years ago

What the.... A couple tweaks here and there and.. At least I have the "TCPNJE" device online to RSCS... It's not connecting yet, but that must be something else. But all the module loading and such is resolved (at least under linux).

ivan-w commented 4 years ago

Auxiliary question: Would the VM/370 NJE modification support CTC instead of BSC?

ivan-w commented 4 years ago

So far I am getting from my TCPNJE capable RSCS subsystem this message when connecting to the TCP/NJE driver:

04:02:07 DMTTNE945E Protocol error detected by link VMF, code 1818 -- link being deactivated
04:02:07 DMTTNE945E Link deactivated -- data being received exceeds expected length
04:02:07 DMTTNE183I Link VMF session terminated
04:02:07 DMTMAN002I Link VMF deactivated
PeterCoghlan commented 4 years ago

One of the thing I ran into is that there is a mixture of the use of "TCPNJE" as if it where a "typedef" and "struct TCPNJE". Possibly some compilers self correct this, but gcc doesn't.

I don't see why this should be an issue. The same is true for SYSBLK, REGS and various other typedefs/structs. Or at least it was true the last time I was able to build this variant.

Once I fixed that, there was only a couple of compilation errors that were easy to fix (mostly the missing definition of the variable "filename" which is implicitly used in a convenience macro.

I expect this is a result of some incompatible code divergence in SDL Hyperion.

Auxiliary question: Would the VM/370 NJE modification support CTC instead of BSC?

No.

ivan-w commented 4 years ago

Nowhere is "TCPNJE" typedef'd... But there are plenty of portion of the code that assume it is. And the use is inconsistent... Sometimes it's "struct TCPNJE", sometimes it's just "TCPNJE".. I mean in the code.

PeterCoghlan commented 4 years ago

Nowhere is "TCPNJE" typedef'd..

It is in htypes.h

ivan-w commented 4 years ago

It's a shame the RSCS NJE mod doesn't support CTC... It'd be closer to the spirit when doing TCP/NJE.

ivan-w commented 4 years ago

Not in the current version.. htypes.h does NOT define TCPNJE as a typedef! Anyway, not the real problem. It'd be cool if it were "consistent". The code contains a mixture of "struct TCPNJE" and "TCPNJE". Either all should be one or the other, but not a mix. Not sure why it's not simply a typedef in tcpnje.h! (and shouldn't be used anywhere else than in tcpnje.c) This is an opaque structure to any other part of hercules isn't it?

ivan-w commented 4 years ago

htypes.h contains stuff that are pertinent to multiple components of hercules or as a whole... "struct TCPNJE" is a control block that is only relevant to the tcp/nje handler... it doesn't (and shouldn't) contain any information that is relevant to any other portions of the engine.

PeterCoghlan commented 4 years ago

It's a shame the RSCS NJE mod doesn't support CTC... it'd be closer to the spirit when doing TCP/NJE

This was always my preferred option. I have an RSCS mod to support CTCA close to working properly but the protocol standard RSCS uses across the CTCA is not sufficient to keep things synchronised between RSCS and a TCPNJE device. BSC lines support ENABLE and DISABLE CCWs which allow additional information to be conveyed between the two.

PeterCoghlan commented 4 years ago

htypes.h contains stuff that are pertinent to multiple components of hercules or as a whole... "struct TCPNJE" is a control block that is only relevant to the tcp/nje handler... it doesn't (and shouldn't) contain any information that is relevant to any other portions of the engine.

This could well be true and if so probably should be fixed. However, TCPNJE is based on COMMADPT and there is a typedef COMMADPT in the htypes.h I am looking at, not far above where typedef TCPNJE is located.

ivan-w commented 4 years ago

The fact that COMMADPT is in htypes.h is a bug I need to fix! It shouldn't be there! I probably made this error 15+ years ago.   :-P   I was starting - made errors... (can't deny it)... Most of this code was written in the early 2000!!!! 8 years earlier I was writing patches to VM or microcode to X.25 and ISDN adapters...

PeterCoghlan commented 4 years ago

The fact that COMMADPT is in htypes.h is a bug I need to fix! It shouldn't be there! I probably made this error 15+ years ago.   :-P   I was starting - made errors... (can't deny it)...

Ivan,

You are being too hard on yourself. I think if you remove typedef COMMADPT from htypes.h you will probably find that anything that includes hstructs.h but does not include commadpt.h (which is nearly everything) will fail to compile.

To deal with this issue properly, struct DEVBLK should be redesigned to allow device type independent information and device type specific information to be properly separated. However, doing that would create all sorts of incompaibilities with everything else out there so this is another of these situations when it would be better to just let sleeping dogs lie rather than try to fix something that isn't really broken.

ivan-w commented 4 years ago

Added a slightly modified version of the TCP/NJE support based on Pete's version. I just changed all reference to the TCPNJE typedef to "struct TCPNJE" and everything compiles just fine.

tcpnje.c and tcpnje.h (and the necessary changes to Makefile.am) where added to the current version (also added a .md version of the README.TCPNJE)

ivan-w commented 4 years ago

Fish, would you be kind enough to integrate it with the Doze build? New files :

If any problem arises please ping me!

PeterCoghlan commented 4 years ago

Ivan, I think the least you could do is to honour the request I made in my first response to this issue, that you remove any indications that I might provide support for this arrangement from the code and documentation. I particularly would like the email address I included removed. It would be even better, if you could remove my name altogether.

Fish-Git commented 4 years ago

Fish, would you be kind enough to integrate it with the Doze build?

10-4.

If any problem arises please ping me!

10-4!

Fish-Git commented 4 years ago

FYI to Ivan (and others):

Main README document:

(which can be reached by scrolling down to the bottom of our main repository web page at https://github.com/SDL-Hercules-390/hyperion).

Notice that there's a "Adding New Files to Hercules" link in the "HERCULES INTERNAL READMEs" section which documents what needs to be done:

You did the *Nix side just fine! You just did't do the Windows (MSVC) stuff, that's all. But I'll take care of it for you this time.

The changes needed for Windows basically involve updating the MSVC makefile fragments (e.g. OBJ_CODE.msvc, MODULES.msvc and MOD_RULES2.msvc) and the various Visual Studio files (e.g. Hercules_VSnnnn.vcxproj, Hercules_VSnnnn.vcxproj.filters, etc). That's about it.

But don't worry about it. I'll take care of that for you this time. I just wanted you (and others) to be aware that there is a documented procedure that explains what needs to be done for the Windows side of things. I created so that you guys can do it yourself without me having to always do it, just in case anything should happen to me.

But as I said, don't worry about it this time. I'll take care of it. I just wanted you to be aware of the documented procedure...

...for next time!   ;-)

I'll go ahead and add a new entry for your new TCPNJE readme to the "Features and Operation" section of the main README too, which it looks like you also forgot to do. But it's no big deal. I'll do it.

Fish-Git commented 4 years ago

Ivan, I think the least you could do is to honor the request I made in my first response to this issue, that you remove any indications that I might provide support for this arrangement from the code and documentation. I particularly would like the email address I included removed. It would be even better, if you could remove my name altogether.

I'll take care of it. At least in the README anyway. I haven't looked at actual source code files yet. Is there anything there I need to get rid of?

ivan-w commented 4 years ago

Peter... I don't get all the crazy stuff.. turning typedef'd TCPNJE names to their original "struct TCPNJE" name is purely cosmetic. The two macro defintions at the begining are just there because this version is only intended for inclusion in the SDL version. There was absolutely no other change in the core of the logic (which is yours)... But hey.. I don't really care - I just want to get something that works! If you want to have a unified version that works on any and all forks that exist, fine by me!

PeterCoghlan commented 4 years ago

Thanks Fish.

Near the end of the banner comment at the top of tcpnje.c

ivan-w commented 4 years ago

I'm not even sure my .md file is github compliant!

PeterCoghlan commented 4 years ago

Ivan,

This has nothing to do with typedefs. I disagree fundamentally with the direction SDL Hyperion has taken on questions of compatibility, portability and documentation of significant changes. I do not want to be appearing to endorsing this direction, I do not want to contribute to this but it appears I have no option on that and I certainly do not want to be providing support for it.

ivan-w commented 4 years ago

Peter,

Fine by me - but I find the direction correct and much more flexible. But that's a question that is irrelevant to the code at hand. the TCP/NJE support is still under an Open Source Initiative license I believe. I see no reason not to keep credit for the work you performed on that particular element. But it is your choice that any reference to you be removed from the current version. I don't think reference to you in that particular piece of code to be an endorsement to the underlying infrastructure.

Fish-Git commented 4 years ago

I'm not even sure my .md file is github compliant!

It mostly was. But I fixed it up for you.   :)

(I haven't committed anything yet but am about to...)

ivan-w commented 4 years ago

Thank you my good friend!

ivan-w commented 4 years ago

I really need to install VScode again on my machine (with the .md linter)

Fish-Git commented 4 years ago

I'm not even sure my .md file is github compliant!

FYI:

I have the PDF saved from the first link. I don't refer to it as often as I used to, but I still do on occasion (like when I need to turn something into a table).

The second link is to a Chrome browser extension that I use to allow me to visually "see" what a .md markdown file is going to actually look like in a browser. Makes it a LOT easier to make sure your markdown is right.

Don't know if there's one for FireFox or not but I would be surprised if there wasn't.

ivan-w commented 4 years ago

I usually use VScode for this now (yes, I am a linux guy - but for my day to day work I use a Windows workstation) - Oh - and they also have a linux version too ;) MS has gone a long way !

Fish-Git commented 4 years ago

I usually use VScode for this now

I've heard of that, yes, but never bothered to look into it. I have VS2008 (still!!) and it works fine for me. I have VS2017 installed in a VMware virtual machine but I don't really use it except for quick tests to make sure Herc still builds okay with it.

MS has gone a long way!

Yep! ... Though I still hate the new VStudio's user interface. Almost as much as I hate the Windows 8 & 10 "Metro" user interface.

I used to make fun (?) of the people that refused to use Windows 7, insisting on sticking with Windows XP instead, but ever since Windows 8 and then 10 came out I changed my tune. I'm sticking with Windows 7 forever! (*)


(*) I haven't purchased it yet but I'm going to pay for extended support and when that runs out, use "0patch".

ivan-w commented 4 years ago

VScode is like.. Eclipse+++++++++ (and is completely free) it's only an IDE and has thousands of plugins for the different languages... Just look vscode on g**gle... download it - takes about 10 minutes to install..

Fish-Git commented 4 years ago

Near the end of the banner comment at the top of tcpnje.c

Haven't committed it yet, but the new banner (last few lines) now looks like this:

/* Original Author of commadpt: Ivan Warren                          */
/* TCPNJE hacked from commadpt by Peter Coghlan and integrated into  */
/* SDL Hercules Hyperion by Ivan Warren.                             */
/*                                                                   */
/* PLEASE DO NOT CONTACT PETER COGHLAN FOR SUPPORT! This is a SDL    */
/* Hercules Hyperion implementation that is NOT supporter by Peter!  */
/* If you need support or wish to report a bug, please do so via     */
/* the official SDL Hercules Hyperions GitHub Issues web page at:    */
/*                                                                   */
/*    https://github.com/SDL-Hercules-390/hyperion/issues            */
/*                                                                   */
/* For more information regarding this implementation please refer   */
/* to our README.TCPNJE document.
/*-------------------------------------------------------------------*/
/* TCPNJE version 1.00                                               */
/*-------------------------------------------------------------------*/

I trust that meets with your approval?

ivan-w commented 4 years ago

Refer to readme/README.TCPNJE.md ;)

PeterCoghlan commented 4 years ago

Thanks Fish. There is a small typo - supporter vs supported but other than that it is fine.

ivan-w commented 4 years ago

Peter, I am still a supporter of your work!

Fish-Git commented 4 years ago

(I haven't committed anything yet but am about to...)

Committed!

(FYI: committed to hyperion-txf too! (*)(**))


(*) It's important to keep our hyperion-txf development repository "in sync" with our main hyperion repository, so that when we eventually release TXF (once it's done and fully tested) by renaming the repositories (hyperion ==> hyperion-old, hyperion-txf ==> hyperion), the end result should be the only change that is introduced are the new TXF related changes, and nothing else. That is to say, we don't want to accidentally lose any changes or fixes we made to our main hyperion repository during our TXF development effort. We don't want to "regress" anything when TXF is eventually released.

(**) Keeping hyperion-txf in sync with hyperion can sometimes be a PITA depending on the change, but if you're careful and make sure all of the "changes" you make (commit) to one you also make (commit) to the other, we should be safe. You might have make your changes/commits to txf manually from time to time (depending on what file was changed and whether it conflicts with any TXF stuff), but such manually synchronization, so far, hasn't been that tough to do.

But that's yet another reason why I do't want our TXF effort to drag on for too long!

Fish-Git commented 4 years ago

STILL TO DO:

Fix the DBGMSG macro and all of the "logmsg"s to WRMSG instead!

(Yeah, it's gonna be a pain! But it's gotta be done too!)

PeterCoghlan commented 4 years ago

Fine by me - but I find the direction correct and much more flexible.

We will have to agree to differ on that then.

But that's a question that is irrelevant to the code at hand. the TCP/NJE support is still under an Open Source Initiative license I believe.

Perhaps but if it becomes part of an SDL Hyperion release, which seems to be the direction things are going, I believe that will be in contravention of the license under which I contributed my work to the Hercules project.

I find it deeply ironic that you are so enthusiastic to make use of my contribution yet you do not want to foster an environment that makes this kind of contribution possible. If SDL Hyperion was the only development environment available to me, I simply would not have been able to write the code I wrote.

I am still a supporter of your work!

How?

Fish-Git commented 4 years ago

STILL TO DO:

Fix the DBGMSG macro and all of the "logmsg"s to WRMSG instead!

Ivan, can you test the attached patch for me? Thanks!

Fish-Git commented 4 years ago

... if it becomes part of an SDL Hyperion release [...] I believe that will be in contravention of the license under which I contributed my work to the Hercules project.

Huh? How so? Please explain!

SDL Hyperion is bound by the same Hercules license that every other Hercules is, or has been, bound to, past, present or future!

If SDL Hyperion was the only development environment available to me, I simply would not have been able to write the code I wrote.

Why the heck not?!   :(

PeterCoghlan commented 4 years ago

... if it becomes part of an SDL Hyperion release [...] I believe that will be in contravention of the license under which I contributed my work to the Hercules project.

Huh? How so? Please explain!

SDL Hyperion is bound by the same Hercules license that every other Hercules is, or has been, bound to, past, present or future!

Well, in the license in question referenced here:

https://github.com/rbowler/spinhawk/blob/master/COPYRIGHT

and here:

https://github.com/SDL-Hercules-390/hyperion/blob/master/COPYRIGHT

(which both seem identical to me), it says (amongst other things):

You may copy and distribute the Software in unmodified form. You may make modifications to the Software and distribute your modifications, in a form that is separate from the Software, such as patches. You may distribute machine-executable forms of the Software or machine-executable forms of modified versions of the Software.

It does not say you may make modifications to the Software and distribute the modified version of the Software.

If SDL Hyperion was the only development environment available to me, I simply would not have been able to write the code I wrote.

Why the heck not?! :(

For one thing, I can't build SDL Hyperion on the hardware and software platforms which I can build Hercules 3.11/3.12/3.13 on and used to develop TCPNJE on.

Fish-Git commented 4 years ago

It does not say you may make modifications to the Software and distribute the modified version of the Software.

I am not a lawyer, but I believe it does:

"You may make modifications to the Software and distribute your modifications...

And as far as "separate from" and "patches" goes:

...in a form that is separate from the Software, such as patches."

I believe using a source code repository that tracks every change made to any module legally qualifies as distributing modifications to the software in the form of patches.

So I honestly cannot see how we are violating the license in any way.  <shrug>

But again, I am not a lawyer.

PeterCoghlan commented 4 years ago

I believe using a source code repository that tracks every change made to any module legally qualifies as distributing modifications to the software in the form of patches.

So I honestly cannot see how we are violating the license in any way.

But again, I am not a lawyer.

I am not a lawyer either but my impression is the same, that having the Software or a modified version of the Software sitting in a source code repository does not violate the license.

Where I forsee a difficulty complying with the license is when a modified version of the Software is packaged into a release and distributed in that manner. As far as I can see, this specific activity constitutes distributing a modified version of the Software where the modifications are not "in a form that is separate from the Software, such as patches" and this activity is therefore not covered by the license.

Fish-Git commented 4 years ago

Where I foresee a difficulty complying with the license is when a modified version of the Software is packaged into a release and distributed in that manner.

How so? Item 4 of the license clearly states that "machine-executable forms of modified versions of the Software" may be distributed (which is what I presume you mean by "release", i.e. "release" = "machine-executable form").

As far as I can see, this specific activity constitutes distributing a modified version of the Software where the modifications are not "in a form that is separate from the Software, such as patches" ...

Quite true.

And your point regarding this is . . . . . . . . . . . . . . . . . . . what?

... and this activity is therefore not covered by the license.

Sure it is! Item 4!

I see absolutely no license violation whatsoever. None. Nada.