[Geany-devel] geany-plugins: Autotools usage

[Chow Loong Jin]

On 23/04/2012 02:14, Chow Loong Jin wrote:
> On 23/04/2012 00:55, Quentin Glidic wrote:
>> On 22/04/2012 18:21, Chow Loong Jin wrote:
>>> On 22/04/2012 20:09, Quentin Glidic wrote:
>>>> Hello,
>>>>
>>>> I noticed a global misuse / misunderstanding of the Autotools 
>>>> (mostly about the dist mechanism) among the geany-plugins.
>>>
>>> Could you elaborate, please?
>>>
>>> I noticed that you have shifted all the ENABLE_$PLUGIN conditionals 
>>> to the top-level Makefile.am instead, by modifying the SUBDIRS 
>>> variable there. However, this is not desirable as disabled plugins 
>>> will be excluded from the release tarball generated by `make dist'.
>>>
>>> --enable/disable-$plugin should only toggle the building of the 
>>> actual plugin.
>>
>> That’s exactly the misunderstanding I was speaking about. From the
>> Automake manual:
>>
>> “If SUBDIRS is defined conditionally using Automake conditionals,
>> Automake will define DIST_SUBDIRS automatically from the possible values
>> of SUBDIRS in all conditions.”
> 
> Great! I hadn't known of the existence of the DIST_SUBDIRS variable. I do recall
> running into issues where whole plugins went missing from the release tarball
> using that approach though.
> 
> Could you include that explanation into the commit message of that patch, please?
> 
>> [...]
>> Automake handles the conditionnal perfectly by itself, and just need
>> some "dist_" prefixes here and there, while EXTRA_DIST should be
>> reserved for files that are used outside of automake scope.
> 
> Of course. I believe they were mostly used in the past for the AM_CONDITIONAL
> separation.
> 
> Apart from that.. it looks mostly good, but here are a couple of questions/issues:
> - Is there a reason you defined plugin = geanydoc in geanydoc/src/Makefile.am?
>   It doesn't look like it's needed there.
> 
> - "FIXME: CSS?" doesn't look like it's needed in geanygendoc -- there's a rule
>   to generate manual.html from manual.rst and manual.css. (This probably
>   shouldn't be dist'd, but Colomban would probably be in a better position to
>   answer that)
> 
> - What's up with the FIXME in geanyvc/src/Makefile.am? I don't think "…" as a
>   FIXME message is particularly descriptive.
> 
> - The following hunk is really unnecessary. I personally prefer spaces between
>   # and the actual comment.
> 
>> diff --git a/pretty-printer/Makefile.am b/pretty-printer/Makefile.am
>> index 8c643f8..a82e819 100644
>> --- a/pretty-printer/Makefile.am
>> +++ b/pretty-printer/Makefile.am
>> @@ -1,4 +1,4 @@
>> -# include $(top_srcdir)/build/vars.auxfiles.mk
>> +#include $(top_srcdir)/build/vars.auxfiles.mk
>>
>>  SUBDIRS = src
>>  plugin = codenav
> 
> 
> And finally... please separate the patch a little. Specifically, I think the
> unittest-related changes should go into their own commit.

One more issue:

hyperair at thinkpwn:~/src/geany-plugins/multiterm/src [master]% make V=1 distdir

                 [ 2:17AM]
rm -f multiterm_la_vala.stamp && echo stamp > multiterm_la_vala.stamp-t
CDPATH="${ZSH_VERSION+.}:" && cd . &&  --vapidir ../../multiterm/src/vapi --pkg
gtk+-2.0 --pkg geany --pkg vte --header multiterm.h --use-header  -C config.vala
context-menu.vala defconf.vala notebook.vala plugin.vala shell-config.vala
tab-label.vala terminal.vala
/bin/bash: --vapidir: command not found


I think we have a bug in Automake. You can reproduce this using:
./configure --disable-addons --disable-autom4te.cache --disable-build
--disable-codenav --disable-debugger --disable-devhelp --disable-geanydoc
--disable-geanyextrasel --disable-geanygdb --disable-geanygendoc
--disable-geanyinsertnum --disable-geanylatex --disable-geanylipsum
--disable-geanylua --disable-geanymacro --disable-geanyminiscript
--disable-geanynumberedbookmarks --disable-geanypg --disable-geany-plugins-1.22
--disable-geanyprj --disable-geanysendmail --disable-geanyvc --disable-gproject
--disable-multiterm --disable-po --disable-pretty-printer --disable-shiftcolumn
--disable-spellcheck --disable-tableconvert --disable-treebrowser
--disable-updatechecker --disable-webhelper --disable-xmlsnippets


Cheers

-- 
Loong Jin

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <http://lists.geany.org/pipermail/devel/attachments/20120423/31800385/attachment.pgp>