Skip to content
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

Update Smack to 4.2 #306

Closed
DanScharon opened this issue Dec 5, 2016 · 18 comments
Closed

Update Smack to 4.2 #306

DanScharon opened this issue Dec 5, 2016 · 18 comments
Assignees
Milestone

Comments

@DanScharon
Copy link

DanScharon commented Dec 5, 2016

Volunteer(s) needed to update Jitsi to use Smack 4.2.

Other issues, e.g. #205 and #300 depend on this.

There is already a branch to start from: https://github.com/jitsi/jitsi/tree/smack4-2

@Flowdalic
Copy link

With my Smack maintainer hat on: I'm happy to assist. We have just recently upgraded Spark from an ancient Smack version (< 4.0) to Smack 4.1.9. That experience told us that

  • The longer you wait, the harder it gets
  • You should expect some regressions
  • Use a build system that is able to consume Maven artifacts.

So you probably want an extended beta period where Jitsi+Smack4.2 is tested, while the changes to the stable Jitsi variant are kept at a minimum. And then do the switch.

Also you really should consider using a build system which is able to consume Maven artifacts. Smack is highly modular these days, and has also many dependencies, of which some also have dependencies. So there are dependencies and transitive dependencies everywhere, which you don't want to add manually as jars. I personally would consider the current build system of Jitsi a blocker of the Smack version bump, and switch to something else (Maven, Gradle, sbt, …) first. I now it's not easy, but IMHO definitely worth it.

@ibauersachs
Copy link
Member

Thanks Florian. We'll try to release 2.10 around Fosdem, which opens the window for a migration. Maybe you're around at Fosdem and we can discuss some of the issues you ran into?

  • Except for Jitsi Desktop, all other projects are already using Maven. It makes sense to migrate Jitsi Desktop too, but this will be a time consuming task which I'm not sure is worth it as a whole. It might make more sense to e.g. embed Ivy into Ant than lots of Ant scripts into the maven-ant-plugin. But I'm open to suggestions.
  • Modularity of Smack: the effort of getting Jitsi into Debian is ongoing. Is Smack available there as one or multiple packages to depend on? If not, any plans to package it?

@Flowdalic
Copy link

Flowdalic commented Jan 21, 2017

Maybe you're around at Fosdem and we can discuss some of the issues you ran into?

I'll be at FOSDEM. Looking forward seeing you there.

@ibauersachs
Copy link
Member

The branch smack4-2 compiles against Smack 4.2-rc2. It won't work yet, I haven't even tried to launch Jitsi. tbc.

@reavertm
Copy link

Hopefully this will solve issue with our corporate Cisco Jabber server (we use custom SSO forwarder/proxy to plug jitsi there) introduced with ecf3954, which results with "IQ must be of type 'set' or 'get'..." exception.

@Flowdalic
Copy link

Hopefully this will solve issue with our corporate Cisco Jabber server (we use custom SSO forwarder/proxy to plug jitsi there) introduced with ecf3954, which results with "IQ must be of type 'set' or 'get'..." exception.

Thanks for your feedback. But without further, more detailed information chances are low that anyone will be able to answer this. Do you have reported the issue? Was there a bug/issue created somewhere? Do you have an XMPP trace which was collected when this happened?

@reavertm
Copy link

reavertm commented Apr 13, 2017

Backtrace starts with:

[java] 2017-04-13 17:36:32.672 SEVERE: [535] org.jivesoftware.smack.Connection.notifyListener() IQ must be of type 'set' or 'get'. Original IQ: <iq id="JXr0u-2" from="name.surname@host.com/jitsi" type="result"><query xmlns="jabber:iq:roster"><item jid="name.surname@host.com" name="name.surname@host.com" subscription="to"><group>Contacts</group></item><item jid=

then follows with a lot of <item> ..</item>... for each contact, ends with </query></iq>. Backtrace:

[java] at org.jivesoftware.smack.packet.IQ.createErrorResponse(IQ.java:165)
[java] at org.jivesoftware.smack.Roster$RosterPacketListener.processPacket(Roster.java:828)
[java] at org.jivesoftware.smack.Connection$ListenerWrapper.notifyListener(Connection.java:877)
[java] at org.jivesoftware.smack.PacketReader$ListenerNotification.run(PacketReader.java:403)
[java] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
[java] at java.util.concurrent.FutureTask.run(FutureTask.java:266)
[java] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
[java] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
[java] at java.lang.Thread.run(Thread.java:745)

Do you suspect it's smack API misuse (so that I should report it here) or smack lib issue (so I should report it upstream)?

@pdefert
Copy link

pdefert commented Jun 20, 2017

Is there a build to do tests?

@ibauersachs
Copy link
Member

I'll push some updates tonight, but I can guarantee you that it won't work for much more than being shown online.

@ibauersachs
Copy link
Member

I've updated the branch. As I said, don't expect much. I need to update some dependent libraries to Smack 4.2 first (i.e. Jingle Nodes, maybe some others too).

@ibauersachs
Copy link
Member

@Flowdalic In case someone is interested in JingleNodes for OpenFire: https://github.com/jitsi/jinglenodes/tree/jitsi-smack4-2

@Flowdalic
Copy link

Thanks @ibauersachs :)

Also FYI there is a bug in 4.2.0 which in rare cases makes Smack not detect IQ replies. If you see ResponseTimeoutExceptions as result of an IQ request while seeing the reply in the debugger, then it's likely that you hit that. It's already fixed in 4.2.1-SNAPSHOT.

@ibauersachs
Copy link
Member

Yai, chat and a basic Jingle call is working.

@Flowdalic Thanks for the IQ reply hint, haven't seen or noticed it yet. What I've run into a few times now however is the bug mentioned by @bbaldino in igniterealtime/Smack#124.

I tried to add your XMPP contact as I'd have some design questions. Any chance you've got some time to chat about these?

@pdefert
Copy link

pdefert commented Jun 29, 2017

and file transfert works with good speed on local network ?

@ibauersachs
Copy link
Member

@pdefert This doesn't have anything to do with file transfers. It may become easier to implement file transfer over Jingle with the update.

@ibauersachs
Copy link
Member

@reavertm see #362

@ibauersachs ibauersachs added this to the 2.14 milestone Nov 1, 2017
@marclaporte
Copy link
Contributor

@GNUDimarik
Copy link
Contributor

@ibauersachs and @damencho can this issue be closed by merging of smack4-2 to masteR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants