Bug 235715

Comment 1 Koichiro Iwao 2019-02-13 15:46:02 UTC

Samy, added you to CC list FYI.

(In reply to Koichiro Iwao from comment #1)

Nice catch, thank you. I will definitely work on that one too.

Comment 3 Koichiro Iwao 2019-02-13 16:20:55 UTC

(In reply to Samy Mahmoudi from comment #2)

We must test both NLS=on/off when adding NLS option. I didn't notice 'cause i didn't test NLS=off.

Created attachment 201993 [details]
Patch file generated with svn diff

(In reply to Koichiro Iwao from comment #3)

As I mentioned previously (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230908#c7), I need to spend more time to make NLS a real option. I suggest to revert the addition of that option so that users who globally unset NLS can still build.

The explanation is that the upstream configure script does not honor --disable-nls properly, so that setting NLS to off has no real effect at configure time and gettext/intltool are still needed.

- Add USES=gl and update USE_GL
- Add USES=gnome
- Revert the addition of NLS
- Reorder variables to pet portlint

Created attachment 201994 [details]
Poudriere log

I had already noticed I/O warnings (apparently related to GCONF_SCHEMAS and deletion) some time ago. These were also generated at the end of this log file.

After fixing the other compiz ports by reverting NLS, I may dive into GCONF_SCHEMAS internals...

Comment 6 Koichiro Iwao 2019-02-14 00:22:34 UTC

(In reply to Samy Mahmoudi from comment #4)

> The explanation is that the upstream configure script does not honor --disable-nls properly, so that setting NLS to off has no real effect at configure 

No worries, do make -VCONFIGURE_ARGS with NLS=on/off. Options framework explicitly sets opposite options.

(In reply to Koichiro Iwao from comment #6)

make -VCONFIGURE_ARGS does not reveal much:
• with NLS on,  the configure script is launched with --enable-nls
• with NLS off, the congigure script is launched with --disable-nls
I think this is the expected result.

Besides, the configure script does not take any action when launched with the --disable-nls argument. I think this is erroneous: gettext/intltool checks should be disabled, translations should not be merged, *.mo files should neither be produced nor installed, etc.

This looks like an upstream problem: the Compiz team used to distribute their source packages with autotools, but they did not use a proper AM_CONDITIONAL to make gettext/intltool optional.

The first (and ideal) solution to this problem would be to submit a patch upstream so that they regenerate their source package with autotools. But compiz is dead (to such a point that sources are currently fetched from the FreeBSD distribution cache)[1].

The second solution would be to do the job in-place: patch configure.ac and the *.am files to regenerate configure and the make files, at the cost of adding autotools to the build dependencies. We would also get rid of runtime dependencies gettext/intltool when NLS is off.

[1] There is, however, a community around compiz-reloaded and I have already submit changes to the build system to increase portability and eventually port compiz-reloaded to FreeBSD (https://gitlab.com/compiz/compiz-core/merge_requests/121). I think they do not have the time to review my work, so I am seriously thinking to create a new port with the changes done within the FreeBSD port system and to resubmit the changes upstream, independently, in small chunks to ease in review.

(In reply to Samy Mahmoudi from comment #7)

"I have already submit" should be "I have already submitted", sorry for that.

Comment 9 Koichiro Iwao 2019-02-14 22:35:22 UTC

Added swills@ to CC list.

(In reply to Samy Mahmoudi from comment #7)

"We would also get rid of runtime dependencies gettext/intltool when NLS is off" should be "We would also get rid of buildtime dependency intltool and runtime dependency gettext when NLS is off". Sorry again.

Hi all,

On the one hand, I should have reverted the introduction of that NLS option, since setting NLS to off prevent the build from succeeding. In addition, as stated in comment 7, gettext can not be disabled easily at the moment.

On the other hand, I have made Compiz Reloaded migrate from intltool to gettext (upstream):

https://gitlab.com/compiz/compiz-plugins-main/-/merge_requests/82

Still WIP, but we now may be able to disable gettext more easily so that NLS=off becomes functional.

(In reply to Samy Mahmoudi from comment #11)
> I should have reverted the introduction of that NLS option
Actually, I have already reverted the introduction of that NLS option (see the patch of comment 4).

Chris, could you review this patch?

(In reply to Samy Mahmoudi from comment #12)
Would love to, and thanks for your time on this.
I'll be out today. But can get to it tomorrow.

Thanks again, Samy.

--Chris

OK I had a chance to check out the diff you
submitted, Samy.
I'm afraid the (port) revision you're working
against is too old, and won't apply.
The current revision is @ 538818 / 2020-06-14
whereas yours is @ 492858

Looking at the differences. I think the current
revision should work fine -- WFM. :-)

Please try building against the current revision,
and tell me if you still have any problems. As I
said, I don't see any here. But it's important to
know if it still doesn't work for you.

Thanks for all your time, and efforts, Samy! :-)

--Chris

(In reply to Chris Hutchinson from comment #14)

Hi Chris,

I just found out that I still had this patch in one of my ports tree, so I realized I forgot to answer your last comment...

Unfortunately, you are absolutely right and the patch does not apply anymore.

The reason is that I intended to comply with the new USE_GL format [1] and I submitted my patch before revision 517000 [2]. Before that revision, the patch was applicable but it did not get committed. At some point, Niclas Zeising made all the remaining faulty ports to comply with the new format [2], and after this revision my patch has become inapplicable. This revision did not require to bump PORTREVISION. To check if the patch would still apply, I just looked at PORTREVISION, which was a mistake!

[1] https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=230692#c10
[2] https://svnweb.freebsd.org/ports?view=revision&revision=517000

Created attachment 218654 [details]
Revert addition of NLS (breaks building when NLS is off)

- Replace 'deprecated' USE_GL=yes [1] with USE_GL=glu
- Revert the addition of NLS
- Reorder variables to pet portlint

[1] https://svnweb.freebsd.org/ports?view=revision&revision=479445

I also replaced the CPPFLAGS and LDFLAGS with USES:localbase, so the log would rather be:

- Replace 'deprecated' [1] USE_GL=yes with USE_GL=glu
- Replace CPPFLAGS and LDFLAGS with USES=localbase to avoid the port to link with
its own libraries in LOCALBASE (if installed) instead of WRKSRC [2]
- Revert the addition of NLS
- Reorder variables to pet portlint

[1] https://svnweb.freebsd.org/ports?view=revision&revision=479445
[2] https://svnweb.freebsd.org/ports?view=revision&revision=423014

We have to repeat the second change in other compiz ports: at least compiz-plugins-extra and compiz-plugins-unsupported are affected.

For your information, here is an interesting discussion about LDFLAGS/LIBS that happened four years ago:
 
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=212987

In our case, using LIBS seems better than LDFLAGS:
I noticed for sure than some compiz ports link with their own libraries in LOCALBASE instead of WRKSRC. I assume the problem is caused by using LDFLAGS instead of LIBS, but this is just an assumption. We need to test that with poudriere, with a proper scientific control (e.g. USES=localbase:ldflags for simplicity).

(In reply to Samy Mahmoudi from comment #18)
Correction: with poudriere ---> with and without poudriere