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

app-text/dblatex: Bump to 0.3.10 version which also fixes \@xmultirow macro issue, bump kicad-doc now that it builds #4991

Closed
wants to merge 1 commit into from

Conversation

zpuskas
Copy link
Contributor

@zpuskas zpuskas commented Jun 26, 2017

Gentoo bug: #614554

Package-Manager: Portage-2.3.6, Repoman-2.3.2

@gentoo-repo-qa-bot gentoo-repo-qa-bot added self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). labels Jun 26, 2017
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull Request assignment

Areas affected: ebuilds
Packages affected: app-text/dblatex

app-text/dblatex: @zpuskas, @gentoo/proxy-maint

@zpuskas zpuskas changed the title app-text/dblatex: Bump to 0.3.10 version which also fixes \@xmultirow… app-text/dblatex: Bump to 0.3.10 version which also fixes \@xmultirow macro issue, bump kicad-doc now that it builds Jun 26, 2017

DEPEND=">=app-text/asciidoc-8.6.9
>=app-text/dblatex-0.3.10
app-text/texlive:=[l10n_en?,l10n_fr?,l10n_it?,l10n_ja?,l10n_nl?,l10n_pl?]
Copy link
Member

Choose a reason for hiding this comment

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

@gentoo/tex, isn't this kind of dependency 100% wrong?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, definitely

app-text/texlive is for users to pull texlive with their useflags

ebuilds should depend on the dev-texlive/pkg they need

>=app-text/dblatex-0.3.10
app-text/texlive:=[l10n_en?,l10n_fr?,l10n_it?,l10n_ja?,l10n_nl?,l10n_pl?]
>=app-text/po4a-0.45
>=sys-devel/gettext-0.18
Copy link
Member

Choose a reason for hiding this comment

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

Any reason not to sort them? It's easier to check them when they're in order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

RDEPEND=""

src_prepare() {
DOCPATH="KICAD_DOC_INSTALL_PATH share/doc/kicad"
Copy link
Member

Choose a reason for hiding this comment

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

Is this actually used anywhere else? If not…


src_prepare() {
DOCPATH="KICAD_DOC_INSTALL_PATH share/doc/kicad"
sed "s|${DOCPATH}|${DOCPATH}-${PV}|g" -i CMakeLists.txt || die "sed failed"
Copy link
Member

Choose a reason for hiding this comment

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

sed -e "s|...|&-${PV}|" ...

(& means whole matched thing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL :)

done

# find out which language is requested
for lang in ${LANGS}; do
Copy link
Member

Choose a reason for hiding this comment

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

Since L10N is USE_EXPAND, starting with EAPI 5 you can just query ${L10N}; it's going to contain all enabled flags — i.e. just one flag with your REQUIRED_USE.

DEPEND="${RDEPEND}"

python_prepare_all() {
distutils-r1_python_prepare_all
Copy link
Member

Choose a reason for hiding this comment

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

This goes after patching.

}

python_install_all() {
python_doscript "${S}"/scripts/dblatex
Copy link
Member

Choose a reason for hiding this comment

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

This belongs in python_install since logically you want to install the script for all supported implementations. Even if you have only one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it's in python_install along with python_optimize there are all sorts of errors. I don't want to patch the setup.py for now, maybe in another PR as it's going to take quite some time to debug.

Copy link
Member

Choose a reason for hiding this comment

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

You should be more specific than that to convince me. AFAICS it's an ebuild that's heavily broken.


python_install_all() {
python_doscript "${S}"/scripts/dblatex
python_optimize
Copy link
Member

Choose a reason for hiding this comment

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

Likewise. Though I'm surprised you need it if Python modules are installed via setup.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried moving this too. The setup.py is a bit hacky, which would require quite some work, possibly pushing it upstream. For now let's just let this be.

@@ -10,6 +10,9 @@
<email>proxy-maint@gentoo.org</email>
<name>Proxy Maintainers</name>
</maintainer>
<use>
Copy link
Member

Choose a reason for hiding this comment

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

Misindent.

ImageConverter.__init__(self, imgsrc="svg", imgdst=imgdst)
- self.add_command(["inkscape", "-z", "-D", "--export-%(dst)s=%(output)s",
- "%(input)s"])
+ self.add_command(["rsvg-convert", "-f", "%(dst)s", "-o","\"%(output)s\"",
Copy link
Member

Choose a reason for hiding this comment

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

Is librsvg really unconditionally required? Is there a point in supporting the inkscape option at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The point is that people still can use dblatex without installing inkscape if they want to, since inkscape is bit on the heavier side compared to librsvg.

Copy link
Member

Choose a reason for hiding this comment

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

No, I mean is there a point in not using rsvg all the time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that was the original authors intent and I would like to keep it as an option.

@zpuskas
Copy link
Contributor Author

zpuskas commented Jul 18, 2017

@mgorny ping

dev-libs/libxslt
dev-libs/libxslt
dev-texlive/texlive-fontutils
dev-texlive/texlive-latex
Copy link
Member

Choose a reason for hiding this comment

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

Also misindent.

Copy link
Member

@mgorny mgorny 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 sorry for the delay. I've fixed those minor issues and merged the dblatex bump. However, kicad-doc seems not to install to the path you expect it to, and that one you need to fix.

EAPI="6"
PYTHON_COMPAT=( python2_7 )

inherit distutils-r1 eutils
Copy link
Member

Choose a reason for hiding this comment

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

eutils doesn't seem to be used.

python_install_all() {
distutils-r1_python_install_all
# move package documentation to a folder name containing version number
mv "${D}"/usr/share/doc/${PN} "${D}"/usr/share/doc/${PF} || die "mv doc"
Copy link
Member

Choose a reason for hiding this comment

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

D already contains a trailing slash in EAPI 6.

RDEPEND=""

src_prepare() {
sed -e "s|KICAD_DOC_INSTALL_PATH share/doc/kicad|&-${PV}|g" -i CMakeLists.txt || die "sed failed"
Copy link
Member

Choose a reason for hiding this comment

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

This does not seem to work:

./usr/share/doc/kicad/help/
./usr/share/doc/kicad/help/pl/
./usr/share/doc/kicad/help/pl/pl_editor.pdf
./usr/share/doc/kicad/help/pl/pcbnew.pdf

Besides, it should be PF, not PV.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've debugged this issue and was surprised at what I found:

  • sed works as expected (yeah, could have been PF though, but whatever)
  • KICAD_DOC_INSTALL_PATH is completely ignored in the build file, it seems they intended to use it but never did. The hard coded install path is /usr/share/doc/kicad/help/{LANG}
  • Technically I could patch the CMakeList file in the CMakeModules, however that would still not help much, since kicad itself looks for the help files in the same hard coded location (you can see that in the kicad source code in the file common/searchhelpfilefullpath.cpp.

So I've decided to the the following:

  • remove the src_prepare section, since it's useless
  • open bugs to upstream about the issue

In the mean time we will have to put up with the non Gentoo path for the docs otherwise it breaks help for kicad users.

Package-Manager: Portage-2.3.6, Repoman-2.3.2
@gentoo-repo-qa-bot
Copy link
Collaborator

😞 The QA check for this pull request has found the following issues:

Issues inherited from Gentoo (may be modified by PR):
https://qa-reports.gentoo.org/output/gentoo-ci/e1dc30598/output.html#dev-db/slony1

@zpuskas
Copy link
Contributor Author

zpuskas commented Dec 18, 2017

Ping? I have updated the kicad-doc-4.0.6 diff on this PR.

@gentoo-bot gentoo-bot closed this in 90956fe Jan 5, 2018
@zpuskas zpuskas deleted the dblatex-bump branch February 19, 2018 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
4 participants