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

Update README on how to build + auto-install pkg-config package #39

Closed
wants to merge 1 commit into from

Conversation

mossroy
Copy link

@mossroy mossroy commented Apr 23, 2017

  • fix a few typos

README.md Outdated
@@ -14,7 +14,7 @@ of kiwix.
Most of the compilation steps are handled by the kiwix-build.py
script.

This script has been tested of Fedora 23 and Ubuntu 16.10
This script has been tested on Fedora 23, Ubuntu 16.10 and Debian 8
Copy link
Contributor

Choose a reason for hiding this comment

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

Who assures that this will be always tested on Debian?

Copy link
Author

Choose a reason for hiding this comment

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

Well, I tested on Debian 8 with current version, but will certainly not test on every release.

Copy link
Member

Choose a reason for hiding this comment

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

In this case we should not say it is tested on Debian 8.
This sentence is a kind of implicit contract. It implies that the script is intended to work on Fedora 23 and Ubuntu 16.10 (and that we will fix potential bug)

Copy link
Author

Choose a reason for hiding this comment

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

I understand.
Maybe, we could add another sentence to say that Debian 8 is not officially supported, but has been reported to work after adding ~/.local/bin in the PATH ?

@kelson42
Copy link
Contributor

Thx for the fixes. Kind of waiting that the Android compilation problems are finally fixed with Kiwix build to revamp that README anyway. See #3.

README.md Outdated

```
pip install meson --user # Install Meson
pip3 install meson --user # Install Meson
cp ~/.local/bin/meson <a_directory_in_your_path>
Copy link
Member

Choose a reason for hiding this comment

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

Does ~/.local/bin not already in the $PATH ?
It's seems to be the case on Fedora and Ubuntu. (And that's way pip install binaries in this directory)

Copy link
Author

Choose a reason for hiding this comment

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

Apparently, it has been added for next version of Debian (Stretch) : https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=820856
For Ubuntu, it has been added in 16.10, and backported on 16.04 : https://bugs.launchpad.net/ubuntu/+source/bash/+bug/1588562
So, If we do not want to officially support Debian 8 or earlier versions of Ubuntu, it's not worth mentioning, I guess.

README.md Outdated
@@ -48,16 +51,17 @@ cp ninja <a_directory_in_your_path>
## Buildings

This is the simple one.
You need pkg-config to be installed
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to make kiwix-build install the pkg-config package if needed.
Adding the package name in the COMMON entries of the PACKAGE_NAME_MAPPERS dictionaries in kiwix-build.py will be enough.

Copy link
Author

Choose a reason for hiding this comment

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

All right. I suppose I should add it to debian_native_dyn and debian_native_static, not for the other environments?

@mossroy mossroy changed the title Update README on how to build on Debian 8 Update README on how to build + auto-install pkg-config package Apr 24, 2017
@mossroy
Copy link
Author

mossroy commented Apr 24, 2017

I rebased the commit and tried to take your comments into account

@kelson42
Copy link
Contributor

This is superseeded by #60. I close therefore that one.

@mossroy I have taken parts of your fixes and an other part (the pkg-config) had already been merged to master.

@kelson42 kelson42 closed this Jun 28, 2017
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