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

make init-common.sh compatible with docker build #71

Merged
merged 19 commits into from Jul 14, 2019

Conversation

@rose-a
Copy link
Contributor

commented Sep 9, 2017

In docker build you cannot use umount and sudo.

To allow running the init-qtrpi-minimal.sh script during the build of a docker image, I suggest the following modifications:

Setting the environment variable QTRPI_DOCKER changes the behaviour of init-common.sh.
In particular, it then skips the sanity check (the docker image is built on a fresh linux image anyway). Further it creates the qtrpi directory structure without using sudo.

To see this in action please have a look at the docker image at arose/qtrpi.

Btw: great work on this project so far!

sudo umount $ROOT/raspbian/sysroot-full/sys
sudo umount $ROOT/raspbian/sysroot-full/dev
sudo umount $ROOT/raspbian/sysroot-full/proc
if [[ ! $QTRPI_DOCKER ]]; then

This comment has been minimized.

Copy link
@synapticvoid

synapticvoid Sep 9, 2017

Member

The variable QTRPI_DOCKER should be defined in common.sh. This is used to centralized all the environment variables in one place.

You could add something like:

USE_DOCKER=$QTRPI_DOCKER

This comment has been minimized.

Copy link
@rose-a

rose-a Sep 9, 2017

Author Contributor

ok, I changed that...

@synapticvoid synapticvoid self-assigned this Sep 9, 2017

@synapticvoid

This comment has been minimized.

Copy link
Member

commented Sep 9, 2017

Hi Alexandre!

Thanks a lot for your contribution, Docker makes perfect sense. There's a couple of things I'd like to discuss before integrating this patch:

  • Don't you think we should provide a Dockerfile at the root of the project? This would automatize the docker initialization.
  • Can you update your PR to the branch develop? The master branch is used only for official releases (we follow the git flow release style)

@rose-a rose-a changed the base branch from master to develop Sep 9, 2017

@rose-a

This comment has been minimized.

Copy link
Contributor Author

commented Sep 9, 2017

Hi Robin,

thanks for your feedback! I made the requested changes so far.

I don't think it makes sense to put one single Dockerfile in the project root, since the configuration has to be hardcoded into the Dockerfile to allow convenient usage.

If it is to be done "right", I'd suggest providing a dockerfile for each possible combination between hardware and software and have them autobuild on docker hub. This way people could make use of the project by simply pulling and running the respective container.

Currently I have made 2 Dockerfiles for the Raspi 3 and Qt 5.6 or 5.7 at https://github.com/rose-a/qtrpi-docker-image, which are automatically build on Docker Hub. I am still working on shrinking the images, since they are currently 4.8 GB in size.

I'll integrate them into the repository as a start if you like.

@synapticvoid

This comment has been minimized.

Copy link
Member

commented Sep 10, 2017

Hi Alexander,
I see what you mean about the Docker files, I'll take some time next week to test qtrpi with the docker configuration you prepared. If everything is right, I'll integrate your PR.

Thanks a lot for your contribution!!

GuillaumeLazar and others added some commits Dec 17, 2017

Merge pull request #89 from jczacharia/master
Unmount loop0 before use
Added qtgraphicaleffects
@GuillaumeLazar

This comment has been minimized.

Copy link
Member

commented Sep 20, 2018

@synapticvoid : this PR should be closed if no one is working on it.

@synapticvoid
Copy link
Member

left a comment

Nice work, merged with such a long delay 😅 !

@synapticvoid synapticvoid merged commit 112e656 into neuronalmotion:develop Jul 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.