Skip to content

Dockerfile: add missing Openthread dependencies#48

Merged
jnohlgard merged 2 commits intoRIOT-OS:masterfrom
jia200x:pr/openthread_dependencies
Oct 5, 2018
Merged

Dockerfile: add missing Openthread dependencies#48
jnohlgard merged 2 commits intoRIOT-OS:masterfrom
jia200x:pr/openthread_dependencies

Conversation

@jia200x
Copy link
Member

@jia200x jia200x commented Aug 29, 2018

This PR adds libtool and m4, required for building OpenThread.
I tried to build OpenThread with and without this PR, and this fixes the issue.

It was rebased into #47
Since #50 got merged, this should work now!

@Hyungsin
Copy link

Hyungsin commented Oct 4, 2018

@miri64 Please review this PR. This is crucial to merge OpenThread.

@miri64
Copy link
Member

miri64 commented Oct 4, 2018

LGTM, but I am not very experienced with docker and I'm not sure about the MIPS toolchain change... @cladmi can you have a look?

Copy link
Member

@jnohlgard jnohlgard left a comment

Choose a reason for hiding this comment

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

Depends on #47, please hold

Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

ACK

@miri64
Copy link
Member

miri64 commented Oct 4, 2018

@jia200x can you rebase to current and make this change independent of #47? #50 fixed that the URL is not reachable.

@cladmi
Copy link
Contributor

cladmi commented Oct 4, 2018

I tested RIOT-OS/RIOT#9336 and it indeed fails with the current image.
I am building this one to verify it makes it work.

@jia200x jia200x force-pushed the pr/openthread_dependencies branch from 85a9e40 to 91bc28a Compare October 4, 2018 10:21
@jia200x
Copy link
Member Author

jia200x commented Oct 4, 2018

@miri64 rebased, now this change is independent of #47

@cladmi
Copy link
Contributor

cladmi commented Oct 4, 2018

Compiling worked with the branch before rebase.

Could you add in the commits message body why you add the dependency ?
Because here the description say as low amount of information as the added line.

@jnohlgard jnohlgard dismissed their stale review October 4, 2018 12:31

#50 was merged

@jnohlgard
Copy link
Member

Looks fine to go

@jnohlgard jnohlgard merged commit dbeabe5 into RIOT-OS:master Oct 5, 2018
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.

6 participants