lxterminal: init at 0.3.1#33240
Conversation
GrahamcOfBorg
added
10.rebuild-darwin: 0
pkgs/top-level/all-packages.nix
Outdated
|
|
||
| findXMLCatalogs = makeSetupHook { } ../build-support/setup-hooks/find-xml-catalogs.sh; | ||
|
|
||
| buildSingleXMLCatalog = makeSetupHook { |
There was a problem hiding this comment.
This now has a name parameter we should be using.
| echo '</catalog>' >> catalog.xml | ||
| } | ||
|
|
||
| if [ -z "$buildSingleXMLCatalogHookDone" ]; then |
There was a problem hiding this comment.
You don't want this. Environment Hooks are functions applied to every dependency, but buildSingleXMLCatalog takes no argument. Just remove the whole if block and call the function manually (e.g. in preConfigure of the package you are packaging).
| # Creates a single XML catalog that references the catalogs found by | ||
| # findXMLCatalogs. | ||
| # Useful for situations where only one catalog is expected. | ||
| buildSingleXMLCatalog() { |
There was a problem hiding this comment.
Probably better if this takes a single argument which is the output file produced.
Ericson2314
left a comment
Ericson2314
left a comment
There was a problem hiding this comment.
Oops sorry that was supposed to be a "request changes" review.
|
|
||
| configureFlags = [ | ||
| "--enable-man" | ||
| "--with-xml-catalog=../catalog.xml" # Generated by buildSingleXMLCatalog |
There was a problem hiding this comment.
Alternately, it is possible to patch the m4 file to autodetect the catalog from the variable, see for example gnome3.cheese.
There was a problem hiding this comment.
Good to know, that makes things a lot easier.
There was a problem hiding this comment.
The only issue is, the macro is in acinclude.m4 instead of the usual m4/gtkdoc_jh_check_xml_catalog.m4.
There was a problem hiding this comment.
Right, and the m4 file has different contents as well. I ended up generating my own patch.
The patch removes the need for a separate script to combine existing catalog files, making the package simpler.
|
Following the advice of @jtojnar I've removed the XML catalog combining setup hook in favor of a solution that patches the m4 file to respect the XML_CATALOG_FILES environment variable. @Ericson2314: This sidesteps the problems you had found with the setup hook solution. Please take a look. |
Ericson2314
left a comment
Ericson2314
left a comment
There was a problem hiding this comment.
Yeah that was a great idea. Looks good!
Janik-Haag
added
the
12.first-time contribution
5 participants
Motivation for this change
LXTerminal is the standard terminal emulator for LXDE, so LXDE users would expect it to exist. Subjectively, it's also a good desktop-agnostic terminal that I use even though I don't use LXDE.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)