Skip to content

Conversation

@pirate
Copy link
Contributor

@pirate pirate commented Apr 22, 2020

No description provided.

Copy link
Contributor

@mgautierfr mgautierfr left a comment

Choose a reason for hiding this comment

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

I'm not sure that a python project should provide a dockerfile to allow user to develop on the project.
Especially if the dockerfile build the libzim and all it dependencies.

  • Python devellopment is a pretty well know process. People use virtual environment not virtual machine/docker.
  • This multiply the libzim's build scripts. This is pretty sure that the dockerfile will be broken when we will change the dependencies of libzim. We have kiwix-build to build our projects, we should reuse it as far as possible.

This is probably a comment relative to the PR #10 (here, this is documentation only) but it seems to me that the way to use "custom" libzim is a bit too complex.
This can easily done by the user with setting CPPFLAGS env variable before running setup.py. setup.py would have nothing to do relative to that.

@pirate
Copy link
Contributor Author

pirate commented Apr 27, 2020

FWIW both @jdcaballerov and I did the entire development of this project in the development docker container (Dockerfile.dev), we don't typically install C++ build environments on our host machines (macOS), and so the development process is made much easier if that docker container is already available. If you really want to remove it, I suggest still making it available somewhere for reference (like the Wiki or bottom of the README or gist maybe).

I also highly recommend keeping the runtime Docker container for python users to use (Dockerfile), it quite simple, it uses the binary release of libzim.so and does not do any compiling.
This will allow people to build things on top of libzim-python without needing a compatible system with libzim.so, and means users can have a simple FROM 'openzim/python-libzim:latest' at the top of their dockerfile and be confident their code will work.

@mgautierfr
Copy link
Contributor

I think that the docker discussion should go in a wider discussion about how we distribute our software/project (docker, simple .so, ...)

For now, I let @kelson42 decide about the dockerfile in python-libzim. I let him push docker files in other repositories without checking them.


@pirate
Merge branch 'master' into documentation

Please rebase the documentation branch on top of master. Do not merge master into documentation

@pirate pirate requested a review from mgautierfr April 29, 2020 00:15
@pirate
Copy link
Contributor Author

pirate commented Apr 29, 2020

This has been rebased, and I removed the custom LIBZIM_LIBRARY_DIR etc env vars in favor of CFLAGS and LDFLAGS everywhere.

@mgautierfr mgautierfr merged commit 754bc7a into master Jun 3, 2020
@mgautierfr mgautierfr deleted the documentation branch June 3, 2020 12:43
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.

3 participants