fixed RuntimeError in setup.py caused by pandoc on non-amd64 arches #820

Merged
merged 1 commit into from Dec 29, 2016

Projects

None yet

3 participants

@WGH-
Contributor
WGH- commented Dec 18, 2016

If pandoc is not installed locally, pwntools' setup.py tells pypandoc to download one.

Unfortunately, it fails on non-amd64 arches because
pypandocs provides precompiled binaries only for amd64.

Since pypandoc is not hard dependency for pwntools,
this error can be skipped without major harm.

In fact, pwntools' setup.py already handles ImportError
gracefully. The problem is, arch mismatch causes
a completely different exception.

@zachriggle
Contributor

So you're running a 32-bit Python interpreter on a 64-bit OS?

setup.py
@@ -78,6 +78,9 @@
long_description = pypandoc.convert_file('README.md', 'rst')
except ImportError:
pass
+except RuntimeError as e:
@zachriggle
zachriggle Dec 19, 2016 Contributor

Change this to handle all exceptions ("Exception as e") and simply print it out and continue.

@WGH-
Contributor
WGH- commented Dec 19, 2016 edited

Actually, I encountered this problem on 32 bit ARM. But it will manifest on any 32 bit machine, obviously.

@zachriggle
Contributor
@zachriggle
Contributor
@WGH-
Contributor
WGH- commented Dec 19, 2016

Then how did you install a 64-bit pandoc? I'm confused.

I never did.

The message printed by pypandoc is somewhat misleading. pypandoc has hardcoded binary packages only for amd64. It (erroneously) assumes only i386 and amd64 ever exist, so instead of actually checking the arch, it checks arch word size. Since arm arch happens to be 32 bit one, the check fails.

https://github.com/bebraw/pypandoc/blob/master/pypandoc/pandoc_download.py#L139

I guess if you would try to call pypandoc.download_pandoc() on 64 bit arm (aarch64), it would try to install package for amd64, which would either fail a bit later or result in non-working installation.

@WGH- WGH- fixed RuntimeError in setup.py caused by pandoc on non-amd64 arches
If pandoc is not installed locally, pwntools' setup.py
tells pypandoc to download one.

Unfortunately, it fails on non-amd64 arches because
pypandocs provides precompiled binaries only for amd64.

Since pypandoc is not hard dependency for pwntools,
this error can be skipped without major harm.

In fact, pwntools' setup.py already handles ImportError
gracefully. The problem is, arch mismatch causes
a completely different exception.
0324ad5
@WGH-
Contributor
WGH- commented Dec 19, 2016

So I changed PR to catch all exceptions instead. Please check.

@zachriggle
Contributor

This sounds like a PyPandoc issue, not a Pwntools issue. Please report this issue to PyPandoc and link to the issue here.

@zachriggle zachriggle added the bug label Dec 19, 2016
@zachriggle zachriggle added this to the Someday milestone Dec 19, 2016
@WGH-
Contributor
WGH- commented Dec 25, 2016

In a way, it's indeed a pypandoc issue. I'll make an issue there.

However, the only way for them to fix it is to raise proper exception (or make binaries for all possible platforms, which sounds unlikely) instead of the one with misleading message. From the pwntools' perspective, nothing will change much, and this PR will still be ok.

@zachriggle
Contributor

Yep! I just want to be able to cross-reference their issue

@zachriggle zachriggle referenced this pull request in bebraw/pypandoc Dec 29, 2016
Open

pandoc_download assumes architecture #125

@zachriggle zachriggle merged commit b584ca3 into Gallopsled:dev Dec 29, 2016

2 of 3 checks passed

codacy/pr Not so good... This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.02%) to 52.72%
Details
@Idolf Idolf modified the milestone: 3.5.0, Someday Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment