Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Fix dockerfile #155

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Fix dockerfile #155

merged 1 commit into from
Apr 21, 2021

Conversation

curquiza
Copy link
Member

@curquiza curquiza commented Apr 21, 2021

docker build and run works now :)

@curquiza curquiza requested a review from MarinPostma April 21, 2021 09:00
@curquiza
Copy link
Member Author

@shekhirin since it changed the dockerfile you updated, let us know if it impacts any of the work you did! 🙂
I checked (build + run), it should not change anything, but maybe I miss something.

@shekhirin
Copy link
Contributor

@curquiza I didn't quite understand the change. We include workspace's root Cargo.lock into the image, but not the project's one from meilisearch-http/Cargo.lock. Why?

I think in order to get reproducible builds it's important to have dependencies locked. Maybe I miss something, though.

@curquiza
Copy link
Member Author

We don't have any Cargo.lock in the workspaces now, so the build of the image currently fails on main.
Maybe @MarinPostma can explain better why we remove them.
In any case, when you build transplant (cargo run), it does not create any new cargo.lock in the workspaces, so for me, the cargo.lock at the root was enough. But maybe I'm wrong.

@shekhirin
Copy link
Contributor

oooh, I got it. Sorry, my fault. Agree with you, yeah!

Copy link
Contributor

@MarinPostma MarinPostma left a comment

Choose a reason for hiding this comment

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

thanks @curquiza !

bors merge

@bors
Copy link
Contributor

bors bot commented Apr 21, 2021

Build succeeded:

@bors bors bot merged commit 662ffc8 into main Apr 21, 2021
@bors bors bot deleted the dockerfile branch April 21, 2021 10:29
@curquiza curquiza mentioned this pull request Jun 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants