Skip to content

WIP: fsrepo: fix musl detection for migrations #3698

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

Merged
merged 2 commits into from
Feb 21, 2017

Conversation

ghost
Copy link

@ghost ghost commented Feb 16, 2017

Note: the tests here are totally incomplete, I keep getting stuck :/

If someone else wants to take a look today, that'd be super helpful. The migrations code itself should be good.


The ldd command used for detection doesn't seem to have a
--version flag on Alpine Linux. It would print the expected
output, but instead of stdout, it would print it on stderr.

The musl detection code would only scan stdout for mentions
of "musl", and would thus not download the musl version
of the fs-repo-migrations executable.

This manifested in the well-known "fs-repo-migrations: not found"
error, which you get when executing something that was linked
against a different libc than the one present on the system.

License: MIT
Signed-off-by: Lars Gierth [email protected]

@ghost ghost added the topic/repo Topic repo label Feb 16, 2017
@ghost ghost added this to the ipfs 0.4.6 milestone Feb 16, 2017
@ghost ghost added the status/in-progress In progress label Feb 16, 2017

test_expect_success "startup fake dists server" '
pretend_server > dist_serv_out &
echo $! > netcat_pid
Copy link
Member

Choose a reason for hiding this comment

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

We should have a setp with kill $(cat netcat_pid) || true

Copy link
Member

Choose a reason for hiding this comment

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

👍 Thanks! forgot that

@whyrusleeping whyrusleeping force-pushed the fix/repo-migration-musl branch from 2b627e1 to faa1d75 Compare February 16, 2017 23:29
@ghost
Copy link
Author

ghost commented Feb 17, 2017

Thanks @whyrusleeping <3 LGTM with kill suggested by kuba

Lars Gierth and others added 2 commits February 17, 2017 08:37
The ldd command used for detection doesn't seem to have a
--version flag on Alpine Linux. It would print the expected
output, but instead of stdout, it would print it on stderr.

The musl detection code would only scan stdout for mentions
of "musl", and would thus *not* download the musl version
of the fs-repo-migrations executable.

This manifested in the well-known "fs-repo-migrations: not found"
error, which you get when executing something that was linked
against a different libc than the one present on the system.

License: MIT
Signed-off-by: Lars Gierth <[email protected]>
@Kubuxu Kubuxu force-pushed the fix/repo-migration-musl branch from faa1d75 to c817f09 Compare February 17, 2017 07:37
@Kubuxu
Copy link
Member

Kubuxu commented Feb 17, 2017

Rebsed to master.

Copy link
Member

@whyrusleeping whyrusleeping left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

2 participants