Update tor-android-service Project to Use Android Toolchain (Firefox 68)

Trac:
Parent Ticket: #30324 (moved)

added TorBrowserTeam201910R component::applications/tor browser ff68-esr owner::tbb-team parent::30324 priority::medium resolution::fixed severity::normal sponsor::44-can status::closed tbb-9.0-must-alpha tbb-rbm type::defect labels

Moving tickets to June

Trac:
Keywords: TorBrowserTeam201905 deleted, TorBrowserTeam201906 added

Moving tickets to July

Trac:
Keywords: TorBrowserTeam201906 deleted, TorBrowserTeam201907 added

Branch 07017

https://github.com/sisbell/tor-browser-build/commit/36adde99982fc4792dbdeeed5faccf165b1cc623

Trac:
Status: new to needs_review

Trac:
Cc: boklm, sisbell to boklm, sisbell, gk

Trac:
Keywords: TorBrowserTeam201907 deleted, TorBrowserTeam201907R added

-org.gradle.jvmargs=-Xmx1536m?

Starting with 9.0 keywords

Trac:
Keywords: N/A deleted, tbb-9.0-must-nightly added

No July any longer.

Trac:
Keywords: TorBrowserTeam201907R deleted, TorBrowserTeam201908R added

Adding Sponsor 44 to ESR68 tickets

Trac:
Sponsor: N/A to Sponsor44-can

Here are some comments:

Where is the remove-vpn.patch coming from? I doubt that it applies to our current HEAD. If we need to have patches for tor-android-service, please provide them against our repo. That's the canonical one now.
I doubt we still need export GRADLE_MAVEN_REPO="file://$gradle_repo" as the patches relying on that are gone?
How do I re-create the dependencies list? I tried now for a while but still failed. Please update the how-to-create-gradle-dependencies-list.txt so I can verify your work.

Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201908R deleted, TorBrowserTeam201908 added

Replying to gk:

Here are some comments:

Where is the remove-vpn.patch coming from? I doubt that it applies to our current HEAD. If we need to have patches for tor-android-service, please provide them against our repo. That's the canonical one now. I'll submit a merge request for the changes. The tor-android-service on my branch has moved the vpn into its own module, so this patch will remove it entirely from the build.

I doubt we still need export GRADLE_MAVEN_REPO="file://$gradle_repo" as the patches relying on that are gone? Good point, we are now using command line to set repo. I'll remove this line.

How do I re-create the dependencies list? I tried now for a while but still failed. Please update the how-to-create-gradle-dependencies-list.txt so I can verify your work.

I'll add those changes now, but basically with gradle 4.10 download logs only show up in debug mode and I have a new grep command since the output is very different.

Change firefox/build export GRADLE_FLAGS="--no-daemon --offline --debug"

cat logs/firefox-android-x86.log | grep "Performing HTTP" | grep -o "https://.*" | sort | uniq > downloads.log

I need to get review and merge of tor-android-service. Branch 0801a contain all the latest changes from orbot to tor-android-service. There are a lot of changes here since I've been keeping up with the work going into Orbot.

https://github.com/sisbell/tor-android-service/commits/0801a

Revision 5c9583cf8e2b7002f71c2971aae7ebb44c067b42 on July 17th is the minimum I think we can use, so that would cut down a lot of review time. The changes after are not so critical for us.

How do we want to do this?

Trac:
Status: needs_revision to needs_review

Replying to sisbell:

I need to get review and merge of tor-android-service. Branch 0801a contain all the latest changes from orbot to tor-android-service. There are a lot of changes here since I've been keeping up with the work going into Orbot.

https://github.com/sisbell/tor-android-service/commits/0801a

Revision 5c9583cf8e2b7002f71c2971aae7ebb44c067b42 on July 17th is the minimum I think we can use, so that would cut down a lot of review time. The changes after are not so critical for us.

Okay. I can see how that particular commit is relevant for us but I am not so sure about the previous ones. What about we pick just that one for now to unblock our nightly and alpha builds and think about the other ones later on?

How do we want to do this?

We can discuss this in one of our team meetings but here is how I think we should do it: we treat tor-android-service as any of our other project. That means proposed changes need a ticket on our bug tracker (right now this is Trac but it might become Gitlab or $foo). Then someone needs to write a patch, point to a branch (be that one on our own infrastructure, or Github, Gitlab or...) and find someone to review and merge the changes.

The patch should a) be based on our master branch as we own the canonical repo now and b) it should be written in a way that we don't have to suddenly apply patches to any of our other projects. E.g. it's nice to redo the VPN support part but the resulting patch should be written in a way that we not have to suddenly apply tor-browser-build patches to rip things out there. More precisely, it would be written in a way that projects can make use of VPN support but others can safely ignore that part to reduce attack surface etc. by setting a compile time option to just not compile that part in in the first place.

Does that make sense? If so, please file new tickets for the changes we should make in tor-android-service, point to the patches (please base them on our master branch to make merging easier) and set the tickets into needs_review.

Trac:
Status: needs_review to needs_revision

Replying to gk:

Replying to sisbell:

I need to get review and merge of tor-android-service. Branch 0801a contain all the latest changes from orbot to tor-android-service. There are a lot of changes here since I've been keeping up with the work going into Orbot.

https://github.com/sisbell/tor-android-service/commits/0801a

Revision 5c9583cf8e2b7002f71c2971aae7ebb44c067b42 on July 17th is the minimum I think we can use, so that would cut down a lot of review time. The changes after are not so critical for us.

Okay. I can see how that particular commit is relevant for us but I am not so sure about the previous ones. What about we pick just that one for now to unblock our nightly and alpha builds and think about the other ones later on?

How do we want to do this?

We can discuss this in one of our team meetings but here is how I think we should do it: we treat tor-android-service as any of our other project. That means proposed changes need a ticket on our bug tracker (right now this is Trac but it might become Gitlab or $foo). Then someone needs to write a patch, point to a branch (be that one on our own infrastructure, or Github, Gitlab or...) and find someone to review and merge the changes.

The patch should a) be based on our master branch as we own the canonical repo now and b) it should be written in a way that we don't have to suddenly apply patches to any of our other projects. E.g. it's nice to redo the VPN support part but the resulting patch should be written in a way that we not have to suddenly apply tor-browser-build patches to rip things out there. More precisely, it would be written in a way that projects can make use of VPN support but others can safely ignore that part to reduce attack surface etc. by setting a compile time option to just not compile that part in in the first place.

Does that make sense? If so, please file new tickets for the changes we should make in tor-android-service, point to the patches (please base them on our master branch to make merging easier) and set the tickets into needs_review.

I think this makes sense but I don't think we would have time before first alpha. What we can do is just use latest tor-android-service from tor repo and I'll modify firefox to use it. It will also require regenerating the dependencies.

I updated the build to use the tor repo version of tor-android-service

https://github.com/sisbell/tor-browser-build/commit/7dbdddb716962ee0ee7de81460762c07d8d772f7 https://github.com/sisbell/tor-browser-build/tree/esr68_0830

Trac:
Status: needs_revision to needs_review

There will also need to be a small change in firefox project: removing the android-jsocks.patch . Once that is done everything will build for the release.

+minVersion=21?

Move must-nightly items to must-alpha ones.

Trac:
Keywords: tbb-9.0-must-nightly deleted, tbb-9.0-must-alpha added

Moving must-alpha tickets to September.

Trac:
Keywords: TorBrowserTeam201908 deleted, TorBrowserTeam201909 added

Trac:
Keywords: TorBrowserTeam201909 deleted, TorBrowserTeam201909R added

require-api.patch obsolete?

We're now in October, moving September outstanding reviews to October

Trac:
Keywords: TorBrowserTeam201909R deleted, TorBrowserTeam201910R added

Replying to cypherpunks:

require-api.patch obsolete?

Yes, when we upgraded the support libraries for esr68, we no longer need this patch.

I'll need to add testing of the require-api patch to confirm its safe to remove. If so, I'll open another issue for that.

Replying to cypherpunks:

+minVersion=21?

minVersion is set to 16 in the gradle command line, overriding the gradle.properties file. So this is ok.

Replying to sisbell:

I'll need to add testing of the require-api patch to confirm its safe to remove. If so, I'll open another issue for that.

That's #31981 (moved).

Are we okay to close this issue?

Replying to sisbell:

Are we okay to close this issue?

Alas, not.

Now that #31568 (moved) is out of the way I gave the patch a fresh look. It seems the disable-daemon.patch is not required to build and run the .apks. What was the reason of including it? Please remove it.

Please file tickets for the proposed tor-android-service changes not picked up, so we can get them reviewed.

Trac:
Keywords: TorBrowserTeam201910R deleted, TorBrowserTeam201910 added
Status: needs_review to needs_revision

The original reason was to avoid the crash when the network was disabled: #31293 (moved). Its no longer needed after that fix.

Replying to gk:

Replying to sisbell:

Are we okay to close this issue?

Alas, not.

Now that #31568 (moved) is out of the way I gave the patch a fresh look. It seems the disable-daemon.patch is not required to build and run the .apks. What was the reason of including it? Please remove it.

Please file tickets for the proposed tor-android-service changes not picked up, so we can get them reviewed.

Removed the disable-daemon.patch

https://github.com/sisbell/tor-browser-build/commit/9bd5cd8b3006eb3f85eae3926d241d1a13b5d998

Trac:
Status: needs_revision to needs_review

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

From a tor-android-service, mavenLocal is not directly needed but it doesn't hurt to add it directly to the gradle build file for tor build.

Replying to gk:

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

Replying to sisbell:

From a tor-android-service, mavenLocal is not directly needed but it doesn't hurt to add it directly to the gradle build file for tor build.

What do you mean? If it's not needed then why do we have the additional complexity? If it is needed then it seems to me supporting the Tor build case directly in tor-android-service is a plus. Other folks might want to build tor-android-service the same way as we.

Replying to gk:

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

I mean that developers who build tor-android-service outside of the tor-browser-build don't need the mavenLocal entry. Since we do need it in tor-browser-build, adding mavenLocal to tor-android-service is the right way to go.

Replying to gk:

Replying to sisbell:

From a tor-android-service, mavenLocal is not directly needed but it doesn't hurt to add it directly to the gradle build file for tor build.

What do you mean? If it's not needed then why do we have the additional complexity? If it is needed then it seems to me supporting the Tor build case directly in tor-android-service is a plus. Other folks might want to build tor-android-service the same way as we.

Replying to gk:

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

Replying to sisbell:

I mean that developers who build tor-android-service outside of the tor-browser-build don't need the mavenLocal entry. Since we do need it in tor-browser-build, adding mavenLocal to tor-android-service is the right way to go.

You mean in our tor-browser-build environment? What about someone who wants to build tor-android-service without network access as we do? Why should that not be supported directly in tor-android-service? This seems to me a valid usecase we should support in general + it would get us rid of a patch: having patches directly in tor-browser-build is considered a bug we should fix, in particular if we own the repository for the respective project as we do with tor-android-service.

Replying to gk:

Replying to sisbell:

From a tor-android-service, mavenLocal is not directly needed but it doesn't hurt to add it directly to the gradle build file for tor build.

What do you mean? If it's not needed then why do we have the additional complexity? If it is needed then it seems to me supporting the Tor build case directly in tor-android-service is a plus. Other folks might want to build tor-android-service the same way as we.

Replying to gk:

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

I'm agreeing with you, let's move mavenLocal to tor-android-service. I'll open the issue on tor-android-service and then remove the patch.

Replying to gk:

Replying to sisbell:

I mean that developers who build tor-android-service outside of the tor-browser-build don't need the mavenLocal entry. Since we do need it in tor-browser-build, adding mavenLocal to tor-android-service is the right way to go.

You mean in our tor-browser-build environment? What about someone who wants to build tor-android-service without network access as we do? Why should that not be supported directly in tor-android-service? This seems to me a valid usecase we should support in general + it would get us rid of a patch: having patches directly in tor-browser-build is considered a bug we should fix, in particular if we own the repository for the respective project as we do with tor-android-service.

Replying to gk:

Replying to sisbell:

From a tor-android-service, mavenLocal is not directly needed but it doesn't hurt to add it directly to the gradle build file for tor build.

What do you mean? If it's not needed then why do we have the additional complexity? If it is needed then it seems to me supporting the Tor build case directly in tor-android-service is a plus. Other folks might want to build tor-android-service the same way as we.

Replying to gk:

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

Replying to sisbell:

I'm agreeing with you, let's move mavenLocal to tor-android-service. I'll open the issue on tor-android-service and then remove the patch.

#32043 (moved) got merged. Could you fix the remaining issue (I think the tickets got filed as children of #32069 (moved)) in tor-browser-build and then we can close the whole Android toolchain upgrade bug, too.

Replying to gk:

Replying to sisbell:

I mean that developers who build tor-android-service outside of the tor-browser-build don't need the mavenLocal entry. Since we do need it in tor-browser-build, adding mavenLocal to tor-android-service is the right way to go.

You mean in our tor-browser-build environment? What about someone who wants to build tor-android-service without network access as we do? Why should that not be supported directly in tor-android-service? This seems to me a valid usecase we should support in general + it would get us rid of a patch: having patches directly in tor-browser-build is considered a bug we should fix, in particular if we own the repository for the respective project as we do with tor-android-service.

Replying to gk:

Replying to sisbell:

From a tor-android-service, mavenLocal is not directly needed but it doesn't hurt to add it directly to the gradle build file for tor build.

What do you mean? If it's not needed then why do we have the additional complexity? If it is needed then it seems to me supporting the Tor build case directly in tor-android-service is a plus. Other folks might want to build tor-android-service the same way as we.

Replying to gk:

Thanks this looks better. Why are we shipping the maven-local.patch and do not add this change to our repo directly? If we add mavenLocal() as last item then there should not be a functional difference to the status quo for projects fetching the dependencies from any of the other specified repositories.

There are still the tickets to file.

Replying to gk:

Replying to sisbell:

I'm agreeing with you, let's move mavenLocal to tor-android-service. I'll open the issue on tor-android-service and then remove the patch.

#32043 (moved) got merged. Could you fix the remaining issue (I think the tickets got filed as children of #32069 (moved)) in tor-browser-build and then we can close the whole Android toolchain upgrade bug, too. I'm not clear which issue you are referring too. Is it related to this bug?

Replying to sisbell:

Replying to gk:

Replying to sisbell:

I'm agreeing with you, let's move mavenLocal to tor-android-service. I'll open the issue on tor-android-service and then remove the patch.

#32043 (moved) got merged. Could you fix the remaining issue (I think the tickets got filed as children of #32069 (moved)) in tor-browser-build and then we can close the whole Android toolchain upgrade bug, too. I'm not clear which issue you are referring too. Is it related to this bug?

We wanted to get rid of the maven-Local.patch in tor-browser-build and use the tor-android-service commit instead that contains the fix I merged yesterday.

Trac:
Status: needs_review to needs_revision

As you said in comment:38

I'm agreeing with you, let's move mavenLocal to tor-android-service. I'll open the issue on tor-android-service and then remove the patch.

The "remove the patch" part is still missing.

Removes two patches we no longer need.

https://github.com/sisbell/tor-browser-build/commit/6480a29f094ebd87833fff6bae56c375b287c08d

Trac:
Status: needs_revision to needs_review

Trac:
Keywords: TorBrowserTeam201910 deleted, TorBrowserTeam201910R added

Looks good, thanks. Cherry-picked to master (commit 11672b3b3a615e7493add5d3b3dae0a7ed960551).

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

closed

mentioned in issue #31568 (moved)

mentioned in issue #30324 (moved)

moved to tpo/applications/tor-browser#30461 (closed)

Update tor-android-service Project to Use Android Toolchain (Firefox 68)

Child items ...

Activity