PASE CLI enhancements - Githubissues

NattyNarwhal commented 2 years ago

better error reporting
allow setting of the control and IPC; control was significant as SSH/local transports had no way to control the behaviour of XMLSERVICE (i.e. *cdata)

Question period:

What's with the CCSID stuff there? We're assuming 1208 (probably for the best), but the EBCDIC CCSID is set to 0.
I'm assuming the 64-bit stuff not working is because of 32-bit PASE requirements somewhere in XMLSERVICE from skimming issues, but why/where?

kadler commented 2 years ago

Can you split out the error reporting to a separate PR? Because of the use of stdin/stdout in xmlservice-cli, I think that needs more contemplation in how the error messages will affect it.

Otherwise, I think the changes look good.

kadler commented 2 years ago

What's with the CCSID stuff there? We're assuming 1208 (probably for the best), but the EBCDIC CCSID is set to 0.

0 is job CCSID. If that's useful to change, maybe we could add another option for that? Unfortunately, we're going to conflict on -c for ctl/ccsid then. (Maybe -e for "ebcdic" or "encoding"?). I'm personally think UTF-8 is the only sensible PASE encoding to use and wouldn't want to add an option to change it, unless someone provided a very strong use-case.

I'm assuming the 64-bit stuff not working is because of 32-bit PASE requirements somewhere in XMLSERVICE from skimming issues, but why/where?

https://github.com/IBM/xmlservice/issues/17 https://github.com/IBM/python-itoolkit/blob/main/docs/api.rst#direct-memory-transport

At the time, we didn't want to depend on the PTF being installed and the bitness of xmlservice-cli doesn't really matter unless you're trying to call in to PASE from whatever ILE code you're calling through XMLSERVICE. Is a 64-bit version something you want/need?

NattyNarwhal commented 2 years ago

Can you split out the error reporting to a separate PR? Because of the use of stdin/stdout in xmlservice-cli, I think that needs more contemplation in how the error messages will affect it.

I've tried to make sure error messages only get written to stderr. I think if something is conflating both output and error, something's wrong with it - besides, it'd have been broken if it got to a point where cli was printing to stderr anyways.

0 is job CCSID. If that's useful to change, maybe we could add another option for that? Unfortunately, we're going to conflict on -c for ctl/ccsid then. (Maybe -e for "ebcdic" or "encoding"?). I'm personally think UTF-8 is the only sensible PASE encoding to use and wouldn't want to add an option to change it, unless someone provided a very strong use-case.

Yeah, I think the default PASE CCSID is fine. I'd suggest maybe using cl builtin in bash and calling chgjob ccsid(n) might be OK for job CCSID, but perhaps inconvenient for i.e. SSH transports.

At the time, we didn't want to depend on the PTF being installed and the bitness of xmlservice-cli doesn't really matter unless you're trying to call in to PASE from whatever ILE code you're calling through XMLSERVICE. Is a 64-bit version something you want/need?

I was looking into calling XMLSERVICE from 64-bit PHP. cli does make a useful testbed to see if it'd even work. The PTF or building the latest XMLSERVICE from source didn't seem to change anything, so I wonder if it actually fixed it or not.

kadler commented 2 years ago

I'd suggest maybe using cl builtin in bash and calling chgjob ccsid(n) might be OK for job CCSID, but perhaps inconvenient for i.e. SSH transports.

So if I understand this, you're suggesting that if the user wanted to change the XMLSERVICE EBCDIC CCSID, they could use the bash builtin to change the job CCSID prior to calling xmlservice-cli and we wouldn't have to add a parameter?

kadler commented 2 years ago

I was looking into calling XMLSERVICE from 64-bit PHP. cli does make a useful testbed to see if it'd even work. The PTF or building the latest XMLSERVICE from source didn't seem to change anything, so I wonder if it actually fixed it or not.

You're looking at making a PHP equivalent of DirectTransport I take it? It should be working, since otherwise DirectTransport wouldn't work. If you want to verify that the PTF has resolved the issue, you can try testing with it.

NattyNarwhal commented 2 years ago

So if I understand this, you're suggesting that if the user wanted to change the XMLSERVICE EBCDIC CCSID, they could use the bash builtin to change the job CCSID prior to calling xmlservice-cli and we wouldn't have to add a parameter?

Yeah, although it'd mean bash overhead every time you'd want to do it.

You're looking at making a PHP equivalent of DirectTransport I take it? It should be working, since otherwise DirectTransport wouldn't work. If you want to verify that the PTF has resolved the issue, you can try testing with it.

I have the source for it, but I haven't had luck with getting it to work even with xmlservice master (since I'm unsure if the system I was developing on had it applied to system xmlservice); I think the PTF might not have fixed it as a result.

NattyNarwhal commented 2 years ago

If you're curious about said code: https://github.com/SeidenGroup/php-ibmi/blob/xmlservice/xmlservice.c

Unfortunately, it just gets 0xF1 return (so, true), but comes back with empty strings - seemingly what I get if I build xmlservice-cli as 64-bit too. It's been a while since I looked into this, so the details are fuzzy here.

kadler commented 2 years ago

Ok, I think things look fine. Can you make the const/style change and appease the DCO bot?

kadler commented 2 years ago

The variables should be const (though C isn't as picky as C++ in this regard it seems). Can you also merge the two commits please?

richardschoen commented 2 years ago

@kadler How soon until that makes the RPMs ? I'm still shipping my customized XMLSERVICE CLI.

kadler commented 2 years ago

Working on it

IBM / xmlservice

PASE CLI enhancements #55