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

Supporting Alpine Linux #1533

Closed
wants to merge 0 commits into from
Closed

Supporting Alpine Linux #1533

wants to merge 0 commits into from

Conversation

arschles
Copy link
Contributor

@arschles arschles commented Dec 3, 2015

I tried to build mc for Alpine Linux (inside an alpine:3.1 docker container). As these instructions say, musl doesn't support the os/user package yet. As this program uses os/user to get the home directory (to get the config file), Alpine builds fail at runtime, even when -C is passed via the CLI.

Right now, this PR removes the entire dependency on os/user, but the final version could use build constraints to toggle usage of os/user. Or perhaps there's an easier way to get this working that I haven't thought of. Thoughts?

arschles added a commit to arschles/deis-builder that referenced this pull request Dec 4, 2015
see minio/mc#1533 for background on the Alpine
Linux compatibility issue. the builder script simply takes advantage of
the -C flag to indicate where the config data lives
@harshavardhana
Copy link
Member

Thanks for trying out 'mc' in docker.

The right fix is to use - https://github.com/minio/minio/blob/8046b24b975a26d659bda12797a30631bb427e8f/utils.go#L63 - which we did with our minio server.

Can you bring this change in this patch?

@harshavardhana
Copy link
Member

Following change needs to be incorporated into your diff. @arschles -

This way it will work in a portable way.

-// user.Current is not implemented on 32bit, falling back and using a workaround instead.
+// user.Current is not implemented on 32bit and statically compiled binaries.
+// Following code provides valid workaround for each of these situations.
 func userCurrent() (*user.User, *probe.Error) {
+       // Set env variable to indicate 'mc' is being run inside DOCKERIMAGE.
+       if os.Getenv("DOCKERIMAGE") == "1" {
+               wd, err := os.Getwd()
+               if err != nil {
+                       return nil, probe.NewError(err)
+               }
+               return &user.User{Uid: "0", Gid: "0", Username: "root", Name: "root", HomeDir: wd}, nil
+       }
        // Remove this check if golang fixes their code to support 32bit properly for user.Current.
        if runtime.GOARCH == "386" && runtime.GOOS == "linux" {
                return &user.User{

arschles added a commit to arschles/deis-builder that referenced this pull request Dec 7, 2015
see minio/mc#1533 for background on the Alpine
Linux compatibility issue. the builder script simply takes advantage of
the -C flag to indicate where the config data lives
arschles added a commit to arschles/mc that referenced this pull request Dec 9, 2015
following this discussion:
minio#1533 (comment)

this change adds support for running binaries built with CGO_ENABLED,
and also adds detection and a workaround for running inside docker
containers
@arschles
Copy link
Contributor Author

arschles commented Dec 9, 2015

@harshavardhana thanks for the direction.

I ported the code that you suggested and it looks like the code already has the pertinent changes in the diff you pasted.

I've also merged from master. Can you please let me know if you prefer I rebase?

arschles added a commit to arschles/deis-builder that referenced this pull request Dec 9, 2015
see minio/mc#1533 for background on the Alpine
Linux compatibility issue. the builder script simply takes advantage of
the -C flag to indicate where the config data lives
@harshavardhana
Copy link
Member

@arschles - can you squash these commits into single change?

@harshavardhana
Copy link
Member

Thanks for your work.

arschles added a commit to arschles/mc that referenced this pull request Dec 9, 2015
https://github.com/minio/minio/blob/8046b24b975a26d659bda12797a30631bb427e8f/utils.go#L63

removing debug output

porting userCurrent code from minio

following this discussion:
minio#1533 (comment)

this change adds support for running binaries built with CGO_ENABLED,
and also adds detection and a workaround for running inside docker
containers

contentType: use contentDB package from server.

fs: Make sure to negate the seeked offset if size is not negative.

Maintainers document updated.
@arschles arschles closed this Dec 9, 2015
@arschles
Copy link
Contributor Author

arschles commented Dec 9, 2015

@harshavardhana I wasn't paying attention to what I was doing and messed up the squash. I'll open a new PR for this change

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

Successfully merging this pull request may close these issues.

None yet

2 participants