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.

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

Add support for the 32-bit PPC Linux platform #30

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

samm-git
Copy link
Contributor

@samm-git samm-git commented May 1, 2015

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

Signed-off-by: Alex Samorukov samm@os2.kiev.ua

Signed-off-by: Alex Samorukov <samm@os2.kiev.ua>
@creack
Copy link
Owner

creack commented Mar 7, 2017

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

@samm-git
Copy link
Contributor Author

samm-git commented Apr 8, 2017

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

@creack creack force-pushed the master branch 3 times, most recently from c7fe15a to 2769f65 Compare June 29, 2019 15:33
@krader1961
Copy link

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

@creack
Copy link
Owner

creack commented Sep 19, 2020

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.

@krader1961
Copy link

@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
Copy link

krader1961 commented Sep 19, 2020

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. 😄

@creack
Copy link
Owner

creack commented Sep 19, 2020

@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.

@samm-git
Copy link
Contributor Author

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

cool. very motivating to do more contributions :-/

@krader1961
Copy link

@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.

@creack creack merged commit 04081dc into creack:master Oct 26, 2023
@creack
Copy link
Owner

creack commented Oct 26, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants