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.

Sign up for GitHub

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

Jump to bottom

Added support for z/OS. #3289

Closed
wants to merge 8 commits into from
Closed

Added support for z/OS. #3289

wants to merge 8 commits into from

Conversation

fswarbrick
Copy link

Added support for z/OS. z/OS is a POSIX compliant mainframe operating system.

JoeKar
JoeKar reviewed May 12, 2024
View reviewed changes
internal/action/actions_posix.go
@@ -1,4 +1,4 @@
// +build linux darwin dragonfly solaris openbsd netbsd freebsd
// +build linux darwin dragonfly solaris openbsd netbsd freebsd zos
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about...

micro/internal/action/terminal_supported.go

Line 1 in 1f51d0b

// +build linux darwin dragonfly openbsd_amd64 freebsd
&

micro/internal/action/terminal_unsupported.go

Line 1 in 1f51d0b

// +build !linux,!darwin,!freebsd,!dragonfly,!openbsd_amd64

...?

Hm curious, solaris and netbsd aren't treated here?!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoeKar, yeah I'm not sure exactly what is going on there. The only use of RunTermEmulator I could find is in the fzf plugin (https://github.com/micro-editor/updated-plugins/blob/master/fzf/main.lua). It checks for shell.TermEmuSupported and if run invokes shell.RunTermEmulator, otherwise invokes shell.RunInteractiveShell. And if shell.RunInteractiveShell fails it invokes (local) fzfOutput.

The plugin works without any source change. I think it's working with RunInteractiveShell. But I really don't understand why.

If I add zos to build for terminal_supported.go and !zos for terminal_unsupported.go I get "Unsupported operating system". Which makes no sense to me, since that message is in terminal_unsupported.go, which it shouldn't be building.

Since it is working with no changes I figured I'd deal with it later. But if you have any ideas, that would be great.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's relevant here...

micro/internal/action/termpane.go

Line 63 in 1f51d0b

if !TermEmuSupported {

...and here too...

micro/internal/action/command.go

Line 965 in 1f51d0b

if !TermEmuSupported {

Have you really build with GOOS being set to zos? According to...
https://go.dev/doc/install/source#environment
...it's isn't really documented, but in...
https://github.com/golang/go/blob/f43d9c40f382def04442898d7581402759bff36a/src/go/build/syslist.go#L32
...it's listed, so I assume it should work in the moment it's invoked with the correct GOOS environment variable for a fitting architecture.

@fswarbrick
Copy link
Author

@JoeKar, I added an additional change.
I'm still working on "terminal" support, but it would be OK to implement the PR as is.

JoeKar
JoeKar reviewed May 16, 2024
View reviewed changes
go.mod Outdated
@@ -5,7 +5,7 @@ require (
github.com/dustin/go-humanize v1.0.0
github.com/go-errors/errors v1.0.1
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
github.com/mattn/go-isatty v0.0.11
github.com/mattn/go-isatty v0.0.16
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, due to the fact that it was introduced with mattn/go-isatty@360bbd2 in v0.0.13 first.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think I should change it to v.0.0.13? I didn't see that in some list I saw somewhere.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually I've no problem to update to the latest tag, but above v0.0.17 we would loose the documented go compatibility of v1.16 due to the dependency of golang.org/x/sys. At least as far as I understood.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the update you should squash your last 5 commits and correct the commit description, since you switched from v0.0.16 to v0.0.13.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not positive, but I think I've done this. (I'm not a power user of git.)

go.mod Outdated
@@ -14,7 +14,7 @@ require (
github.com/zyedidia/clipper v0.1.1
github.com/zyedidia/glob v0.0.0-20170209203856-dd4023a66dc3
github.com/zyedidia/json5 v0.0.0-20200102012142-2da050b1a98d
github.com/zyedidia/tcell/v2 v2.0.10 // indirect
github.com/zyedidia/tcell/v2 v2.0.10
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly unrelated to the PR, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I made this change. I think the build did. Does it have an effect?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to stay with the current content, as long as it isn't really needed here.

@dmaluka:
What's your opinion about it, since you're more familiar in go than me.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really familiar with this particular aspect of Go. But I agree it doesn't look like we should change this tcell line.

@fswarbrick
Revert "z/OS requires github.com/mattn/go-isatty v0.0.16"
8fb9ec0 
This reverts commit f4ba632.
@fswarbrick
Redo last commit.
881833b
@fswarbrick
Revert "Redo last commit."
f002f6f 
This reverts commit 881833b.
@fswarbrick
One more time.
c15d0a0
@fswarbrick
z/OS requires github.com/mattn/go-isatty v0.0.13.
6bb279b 
Squashed commit of the following:

commit c15d0a0
Author: Frank Swarbrick <frank.swarbrick@efirstbank.com>
Date:   Thu May 16 13:50:25 2024 -0600

    One more time.

commit f002f6f
Author: Frank Swarbrick <frank.swarbrick@efirstbank.com>
Date:   Thu May 16 13:40:56 2024 -0600

    Revert "Redo last commit."

    This reverts commit 881833b.

commit 881833b
Author: Frank Swarbrick <frank.swarbrick@efirstbank.com>
Date:   Thu May 16 13:35:17 2024 -0600

    Redo last commit.

commit 8fb9ec0
Author: Frank Swarbrick <frank.swarbrick@efirstbank.com>
Date:   Thu May 16 13:31:41 2024 -0600

    Revert "z/OS requires github.com/mattn/go-isatty v0.0.16"

    This reverts commit f4ba632.

commit f4ba632
Author: Frank Swarbrick <frank.swarbrick@efirstbank.com>
Date:   Wed May 15 14:15:24 2024 -0600

    z/OS requires github.com/mattn/go-isatty v0.0.16
@fswarbrick
Merge branch 'master' of https://github.com/fswarbrick/micro-z-OS
d943dcf
@JoeKar
Copy link
Collaborator

JoeKar commented May 19, 2024 •
edited

Hm, the commit history doesn't look that good so far. Please squash f4ba632..d943dcf (your second till the last) commits together, so only two commits are left.

@fswarbrick
Copy link
Author

@JoeKar, I'm not sure what I am going, git-wise. I am going to delete this PR, recreate my changes anew, and then create a PR. Not today, though. Something I also missed is the changes in some of the dependencies. I will do those first.

@fswarbrick fswarbrick closed this May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmaluka dmaluka dmaluka left review comments

@JoeKar JoeKar JoeKar left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fswarbrick @JoeKar @dmaluka