1161811 - Update Picasso to latest release

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect)

Description

[:Margaret Leibovic]

Nick, what's the process to update Picasso? I can't tell what version we have in the tree now, but it looks like it hasn't changed since it first landed over a year ago.

We'll need to update to at least version 2.4 for bug 1004517, but it looks like they're at 2.5.2 now.

Comment 1

[Nick Alexander :nalexander [he/him]]

(In reply to :Margaret Leibovic from comment #0)
> Nick, what's the process to update Picasso? I can't tell what version we
> have in the tree now, but it looks like it hasn't changed since it first
> landed over a year ago.

Since Picasso is a pure Java library, it's a straight copy from https://github.com/square/picasso/tree/master/picasso/src/main/java/com/squareup/picasso into mobile/android/thirdparty/com/squareup/picasso, and then a simple update of the included files in the relevant moz.build file.

It's updating code and looking for regressions that is tricky here.

Comment 2

[:Margaret Leibovic]

(In reply to Nick Alexander :nalexander from comment #1)
> (In reply to :Margaret Leibovic from comment #0)
> > Nick, what's the process to update Picasso? I can't tell what version we
> > have in the tree now, but it looks like it hasn't changed since it first
> > landed over a year ago.
> 
> Since Picasso is a pure Java library, it's a straight copy from
> https://github.com/square/picasso/tree/master/picasso/src/main/java/com/
> squareup/picasso into mobile/android/thirdparty/com/squareup/picasso, and
> then a simple update of the included files in the relevant moz.build file.

So, I just tried updating the java files we have in the tree to their 2.5.2 release and that resulted in a ton of build errors... how far away are we from just using gradle to import third-party libraries? :)

But in all seriousness, it looks like these are related to some missing packages:

error: package org.jetbrains.annotations does not exist
error: package com.squareup.okhttp does not exist

Do we need to import more libraries for this to work?

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to :Margaret Leibovic from comment #2)
> (In reply to Nick Alexander :nalexander from comment #1)
> > (In reply to :Margaret Leibovic from comment #0)
> > > Nick, what's the process to update Picasso? I can't tell what version we
> > > have in the tree now, but it looks like it hasn't changed since it first
> > > landed over a year ago.
> > 
> > Since Picasso is a pure Java library, it's a straight copy from
> > https://github.com/square/picasso/tree/master/picasso/src/main/java/com/
> > squareup/picasso into mobile/android/thirdparty/com/squareup/picasso, and
> > then a simple update of the included files in the relevant moz.build file.
> 
> So, I just tried updating the java files we have in the tree to their 2.5.2
> release and that resulted in a ton of build errors... how far away are we
> from just using gradle to import third-party libraries? :)

Sad making :/

> But in all seriousness, it looks like these are related to some missing
> packages:
> 
> error: package org.jetbrains.annotations does not exist
> error: package com.squareup.okhttp does not exist
> 
> Do we need to import more libraries for this to work?

Looks like Picasso has acquired some dependencies, so yes.  Although org.jetbrains.annotations is an odd one, since that's just for build-time annotation checking, I think.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Nick Alexander :nalexander from comment #3)

> > error: package org.jetbrains.annotations does not exist
> > error: package com.squareup.okhttp does not exist
> > 
> > Do we need to import more libraries for this to work?
> 
> Looks like Picasso has acquired some dependencies, so yes.  Although
> org.jetbrains.annotations is an odd one, since that's just for build-time
> annotation checking, I think.

This is where I get grumpy and suggest that we don't pull in more libraries that duplicate functionality. If OKHTTP comes in, one of our other HTTP libs should go.

Comment 5

[:Margaret Leibovic]

(In reply to Mark Finkle (:mfinkle) from comment #4)
> (In reply to Nick Alexander :nalexander from comment #3)
> 
> > > error: package org.jetbrains.annotations does not exist
> > > error: package com.squareup.okhttp does not exist
> > > 
> > > Do we need to import more libraries for this to work?
> > 
> > Looks like Picasso has acquired some dependencies, so yes.  Although
> > org.jetbrains.annotations is an odd one, since that's just for build-time
> > annotation checking, I think.
> 
> This is where I get grumpy and suggest that we don't pull in more libraries
> that duplicate functionality. If OKHTTP comes in, one of our other HTTP libs
> should go.

I agree, we do not need that functionality for Picasso. Yesterday I tried manually applying the commit lucasr wrote to support different ways of loading images [1], but it did not apply cleanly at all. I didn't look closely at all the conflicts, but I saw a ton of .rej files, so it would probably take a bit of work to apply this, but that might be the best way forward for the short term.

Unfortunately the first release with lucasr's changes also adds okhttp.

But wait... I just noticed this logic we currently have in the tree:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/home/ImageLoader.java#67

And that handles custom distribution URIs. So I wonder if I could stick some logic in there to handle our other custom URIs that pull images from the jar (that's the main reason we need new logic - for bug 1004517).

[1] https://github.com/lucasr/picasso/commit/4c6cf231e69f665cd70c4676517e8ba2f261b111

Comment 6

[:Margaret Leibovic]

I'm just gonna WONTFIX this, since there's a way to work around the feature I thought we needed to upgrade for.

In the future we can re-assess whether or not it would be worth picking up the extra dependencies for new features.