Bug 41790

Branch at URL attempts to fix.

+    AC_SUBST(NICE_CFLAGS)
+    AC_SUBST(NICE_LIBS)
+else
+    NICE_CFLAGS=
+    NICE_LIBS=
+fi

If you're not AC_SUBSTing the empty variables, can't you just leave the else clause out entirely?

I don't really like how this patch litters quite so many ifdefs around, even down to changing the number of arguments to gabble_file_transfer_channel_new()…

(In reply to comment #2)
> +    AC_SUBST(NICE_CFLAGS)
> +    AC_SUBST(NICE_LIBS)
> +else
> +    NICE_CFLAGS=
> +    NICE_LIBS=
> +fi
> 
> If you're not AC_SUBSTing the empty variables, can't you just leave the else
> clause out entirely?
Actually I should AC_SUBST them every time, will fix this

> I don't really like how this patch litters quite so many ifdefs around, even
> down to changing the number of arguments to gabble_file_transfer_channel_new()…
Tbh, I don't like it either, but I don't see how it could be any better, suggestions?

(In reply to comment #3)
> (In reply to comment #2)
> > +    AC_SUBST(NICE_CFLAGS)
> > +    AC_SUBST(NICE_LIBS)
> > +else
> > +    NICE_CFLAGS=
> > +    NICE_LIBS=
> > +fi
> > 
> > If you're not AC_SUBSTing the empty variables, can't you just leave the else
> > clause out entirely?
> Actually I should AC_SUBST them every time, will fix this
> 
> > I don't really like how this patch litters quite so many ifdefs around, even
> > down to changing the number of arguments to gabble_file_transfer_channel_new()…
> Tbh, I don't like it either, but I don't see how it could be any better,
> suggestions?

I guess it would be clearer if there were something like a subclass of GabbleFTChannel for GTalk (well, maybe GabbleFTChannel would be an abstract base class and there would be two subclasses, one for GTalk and one for SI transfers), maybe?

How about just being able to disable FT completely?

(In reply to comment #5)
> How about just being able to disable FT completely?

Ok, as agreed, we decided to go this route, so hijacking this bug.

URL updated to point to a new branch "ft-optional" that adds an option to disable ft completely.

I tried to use as few ifdefs as possible.

Seems reasonable enough.

How come you didn't just conditional out all the file transfer tests in tests/twisted/Makefile.am instead of skipping each one if FT is not enabled?

(In reply to comment #7)
> Seems reasonable enough.
> 
> How come you didn't just conditional out all the file transfer tests in
> tests/twisted/Makefile.am instead of skipping each one if FT is not enabled?

At first, I made the tests conditional in tests/twisted/Makefile.am, but then I saw other tests that were being skipped due to some disabled stuff, so I did the same as I guess this was intended. See twisted/sidecars.py, jingle/google-relay.py, jingle/call-*, etc.

(In reply to comment #8)
> At first, I made the tests conditional in tests/twisted/Makefile.am, but then I
> saw other tests that were being skipped due to some disabled stuff, so I did
> the same as I guess this was intended. See twisted/sidecars.py,
> jingle/google-relay.py, jingle/call-*, etc.

OK. Merge away.

(In reply to comment #9)
> (In reply to comment #8)
> > At first, I made the tests conditional in tests/twisted/Makefile.am, but then I
> > saw other tests that were being skipped due to some disabled stuff, so I did
> > the same as I guess this was intended. See twisted/sidecars.py,
> > jingle/google-relay.py, jingle/call-*, etc.
> 
> OK. Merge away.

Thanks, merged. It will be in gabble 0.15.4.