Opened 11 years ago

Last modified 10 years ago

#152 new task

Sync Perl module patches for Jabber with upstream

Reported by: andersk@mit.edu Owned by:
Priority: minor Milestone:
Component: jabber Keywords:
Cc: jgross@mit.edu

Description

See PerlModulePatches for a current list of our patches to XML::Stream, Net::XMPP, and Net::Jabber, and their upstream status. We should file bugs for the patches that aren’t upstream, and we should merge the newer upstream versions of XML::Stream and Net::XMPP.

Change History (5)

comment:1 Changed 11 years ago by jgross@mit.edu

  • Cc jgross@mit.edu added

I've cataloged most/all of the changes from between the barnowl versions and the upstream versions (see https://github.com/JasonGross/barnowl/commits/perl-module-sync). I'll start with the Net::Jabber ones, because it's the shortest:

The differences from the upstream version to be merged into the barnowl version (see https://github.com/JasonGross/barnowl/commit/83ee3c7298a983ac7878d33c6a006efc8dc3dc6c) are all either spacing changes or changes from Obj->new(...) to new Obj().

There's a single patch for submission upstream (though the upstream development seems to be dead), made by nelhage (see https://github.com/barnowl/barnowl/commit/d7c2ce6637671aed603983ac425e82418a229bc4).

comment:2 Changed 11 years ago by jgross@mit.edu

For XMPP::Stream, I split the changes from the barnowl version to the newest upstream version into three categories.

The first category is composed of minor changes, such as documentation, licence, spelling, spacing, etc. (https://github.com/JasonGross/barnowl/commit/075c65a5756e3a62838fac4e281939bdcce2a227)

The second category (https://github.com/JasonGross/barnowl/commit/9a8604abfc76cf403aa22fb3ba77449211cc0941)consists of minor changes:

  • A work-around for windows that, while unnecessary for linux/mac barnowl, shouldn't break anything
  • A change that uses $self->{SIDS}->{$sid}->{to} in place of $self->{SIDS}->{$sid}->{hostname} when available -- it seems to me like this shouldn't break anything (given that the developers thought that this should be used in preference to $self->{SIDS}->{$sid}->{hostname}, and fall back on that), but I'm not completely sure
  • a change of $self->get_cdata() to $child->children() -- I'm not sure what this does or why this changed. I don't think it breaks anything, but I haven't checked.

The last category (https://github.com/JasonGross/barnowl/commit/ed7f5649a0804e59bcb1fde4a29a59a1a04d554a) contains only one change, which adds some new things and removes some barnowl things (see commit by asedeno, https://github.com/barnowl/barnowl/commit/ea215acf582ad1dae07bc35879171b3a3f2dad81). I'm not sure if it fixed the problem that commit was intended to fix. I'm not sure how to test thisIf it doesn't, we should submit a patch upstream.

comment:3 in reply to: ↑ description Changed 11 years ago by jgross@mit.edu

The last comment should say Net::Stream, not XMPP::Stream.

Net:XMPP had the largest number of changes (pulling from upstream version 1.02), so I broke it up into five commits.

  • There were a large number of minor refactorings (e.g., changes of Log5 to Log1, moving the location of the init code to a different method in the same file, adding a few minor functions, defining a bunch of debug variables, commenting out a line in an an if statement (this one should be checked to make sure it doesn't break anything)). The second commit consists of these changes (see https://github.com/JasonGross/barnowl/commit/https://github.com/JasonGross/barnowl/commit/).
  • The third commit consists of a bunch of changes of UNIVERSAL::isa($jid,...) to $jid->isa(...), and one more significant change that seems to remove a for loop. I'm not sure if removing the for loop does bad things with references; this should be checked, and, if it breaks things, a patch should be submitted upstream. I'm not sure what UNIVERSAL::* is, but I'm guessing that this is just convention. (see https://github.com/JasonGross/barnowl/commit/https://github.com/JasonGross/barnowl/commit/).

comment:4 Changed 11 years ago by andersk@mit.edu

It looks like you are treating this problem as “pick the right changes from the upstream version to merge into our version”?

This is backwards. We are downstream and they are upstream, so we need to figure out which of _our_ changes to apply to their version.

That’s what I started to figure out at PerlModulePatches, which I linked from the original description of this ticket. Let’s finish that process so we can get a cleanly separated stack of patches on top of upstream, instead of producing a single unreviewable commit on top of the current patched version (which will make things suck for us next time we want to merge a new upstream version).

(In general, two-way merges do not work; you cannot compare patched version A of some software to patched version B, and expect to figure out how to combine them into patched version A+B. You need to do a three-way merge, based on patched version A, patched version B, _and_ the unpatched version that they both descended from.

For example, if you look carefully at the ancestry of the two print statements you mentioned in your last commit, you notice that they were not added by upstream, but rather removed by us. But your methodology provides no way to distinguish those situations.)

As you can see on PerlModulePatches, many of the changes we need were upstreamed in the current development versions of the modules, so we should maybe start with the latest development releases instead of the latest stable releases. The development releases also support SSL CA verification (#139).

comment:5 Changed 10 years ago by jgross@mit.edu

  • Priority changed from major to minor

This will no longer be relevant to BarnOwl when davidben finishes his AnyEvent::XMPP branch.

Note: See TracTickets for help on using tickets.