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
[staging] asciidoc: 8.6.9 -> 9.0.4 #102398
Conversation
Sooooo I successfully built |
Alright, |
chmod +x "$n" | ||
done | ||
'' else '' | ||
sed -e "s|^ENV =.*|ENV = dict(XML_CATALOG_FILES='${docbook_xml_dtd_45}/xml/dtd/docbook/catalog.xml ${docbook_xsl_ns}/xml/xsl/docbook/catalog.xml ${docbook_xsl}/xml/xsl/docbook/catalog.xml')|" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this first bit here shouldn't be needed. if the docbook dependencies are added to nativeBuildInputs you will have the setup hook findXMLCatalogs
which will set this appropriately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dasJ does this need to be addressed still? I don't have enough context to judge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the same as my other comment
sed -e "s|^ENV =.*|ENV = dict(XML_CATALOG_FILES='${docbook_xml_dtd_45}/xml/dtd/docbook/catalog.xml ${docbook_xsl_ns}/xml/xsl/docbook/catalog.xml ${docbook_xsl}/xml/xsl/docbook/catalog.xml')|" \ | ||
-e "s|^XSLTPROC =.*|XSLTPROC = '${libxslt.bin}/bin/xsltproc'|" \ | ||
-e "s|^XMLLINT =.*|XMLLINT = '${libxml2.bin}/bin/xmllint'|" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From looking at the a2x.py it just wants these in PATH. So I think also, these programs being in nativeBuildInputs should be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I don't really know enough about asciidoc to judge this myself, I think we cannot just put them into nativeBuildInputs
since that would require putting them into the nativeBuildInputs
of every build that uses a2x
Oh wait, a2x is a part of asciidoc. So that is needed. |
In that case, it would really be better to do a patch on a2x Edit: How exactly does
I see the description
I would then think, does it matter what version of the docbook catalogs are used? Conversion between formats and being to distinguish the docbook version is probably needed (not familiar with the tool but I can imagine it being used to convert documentation in other projects builds even). |
Yeah, I can do the stuff with the patch. About your question with the catalog and stuff: I don't know. |
Yeah, this is the error when not setting the catalog files:
|
@worldofpeace Still want me to switch to a patch and |
Result of 3 packages built:
|
I'd love to see this advance, anything I can do to help? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pure code review looks good to me
e5312ee
to
dacdb88
Compare
Is it possible to fix the asciidoc package when used in a cross-compiled package? If I'm not wrong it's currently broken.
I'd expect to get the same derivation for asciidoc in both cases. In particular, in the latter derivation the python scripts have aarch64 python shebangs (In fact the build fails somewhere on several nixpkgs commits I tried: so I'm not 100% sure but I remembered this). I suppose handling dependencies without going through the *Inputs scheme (e.g., |
(needs a rebase, it seems) |
Main motivation was to port it to Python3 which is due to NixOS#101964. So: - Switch from asciidoc to asciidoc-py3 - Switch from Python 2 to Python 3 - This needs autoreconfHook now - We also need to do some patching in a2x.py - Switch to patchShebangs (more readable) - Only input w3m if we actually build with it
@raboof Rebased @ThibautMarty good point, sadly I don't really know a lot about cross compiling. Can anyone help out here? Or do you think it might be possible that someone else (with more knowledge in this field) fixes this in a later PR? |
@dasJ there are two left-over comments in the review. Do you mind taking a look? Cross is not officially supported so it can be fixed async. |
I cannot really judge #102398 (comment) since I actually don't really know how it all plays together. I chose the |
Let's merge this and clean up afterwards if any breakage happens. |
@dasJ please keep an eye on the staging channel on hydra :) |
Main motivation was to port it to Python3 which is due to #101964.
So:
Motivation for this change
#101964
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)