pty icon indicating copy to clipboard operation
pty copied to clipboard

Add support for the 32-bit PPC Linux platform

Open samm-git opened this issue 10 years ago • 9 comments

After this change i am able to compile docker for the e500v2 PowerPC CPU target.

Signed-off-by: Alex Samorukov [email protected]

samm-git avatar May 01 '15 19:05 samm-git

Could you provide more detail on how you proceed? Is it using gccgo?

creack avatar Mar 07 '17 14:03 creack

Hi, yes, its using gccgo. I did this to build docker on ppc/32

samm-git avatar Apr 08 '17 17:04 samm-git

This P.R. is old enough that it should probably just be closed without merging.

krader1961 avatar Sep 19 '20 02:09 krader1961

I am going to add some tests building the lib with llvmgo. I am hoping to be able to try the PR then.

The reason it has been open so long is that I haven't been able to compile. If someone has a Dockerfile or Vagrantfile that would setup gccgo with the powerpc chain, it would be awesome.

creack avatar Sep 19 '20 02:09 creack

@creack, As a senior software engineer I appreciate your caution and desire to validate every change. However, this adds a single file to enable building on 32-bit PPC architecture. It appears to be identical to the existing ztypes_ppc64.go file. Minus a // +build ppc line that is implied by the filename but which I would still include for maximum clarity and consistency. I can't see any risk to merging this. Either it correctly allows using this package on 32-bit PPC or it doesn't. The latter case being the current situation; i.e., as if the P.R. wasn't merged.

Having said all that it is possible that 32-bit PPC support is no longer useful enough to warrant even this trivial change. And, in fact, https://golang.org/doc/install/source#environment shows that there is no "ppc" architecture; only "ppc64" and "ppc64le". To use the "ppc" architecture you have to use gccgo. I have no idea how common it is for people to use Go on 32-bit PPC. So it's perfectly reasonable to reject this on a "not worth trying to support 32-bit PPC platforms" basis. But in that case there should probably be something official to that effect in the documentation.

krader1961 avatar Sep 19 '20 03:09 krader1961

I should point out the reason I'm commenting on this P.R. is that I'm about to change the dependency by the Elvish shell from the obsolete kr package to this one. One of the things I always do when deciding on creating a dependency is check whether the project appears to be active or dead. Pull-requests that are more than a year old are worrisome. :smile:

krader1961 avatar Sep 19 '20 03:09 krader1961

@krader1961 I appreciate the feedback. Indeed, merging such a small change seem reasonable, however, there is always a risk. The implied build tag works with known architectures, which might cause a conflict go instead of gccgo, it might conflict with the other ppc architectures.

This library is used by many projects in production, so I am always very careful when merging something.

Closing the PR declining the support for the platform is something I would rather avoid. We have seen some large projects migrating from Go to Python because of the lack of support of x or y arch. It might not be a priority, but eventually, we'll get there, and having the PR open, even if very old, is an easy way for anyone with this particular need to apply the patch before it gets mainstream.

The project is not active, but it is not dead :) Bugs and critical issues get fixed promptly. Most recently, go1.15 introduced a breaking change and a new version of the pty lib has been released before go1.15 (#96 #97). Also, more common arch/os support get added quickly when it is easy to test (solaris, freebsd/arm64, etc). The issue with this PR is the difficulty to setup a working gccgo toolchain for powerpc.

creack avatar Sep 19 '20 11:09 creack

This P.R. is old enough that it should probably just be closed without merging.

cool. very motivating to do more contributions :-/

samm-git avatar Sep 19 '20 19:09 samm-git

@samm-git, Personally I wouldn't be motivated to create another P.R. for a project if a P.R. I created five years ago was never merged and there were no comments explaining what needed to be changed to get it merged.

krader1961 avatar Sep 19 '20 20:09 krader1961

Fair enough. My main reservation was the inability to test as it is not an officially supported platform and requires a custom Go compiler.

That being said, it doesn't seem to break anything and the change is only used in this scenario, ignored by the regular compiler.

creack avatar Oct 26 '23 15:10 creack