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

Forwarding patches from the debian package #1277

Closed
chirayudesai opened this Issue Jun 20, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@chirayudesai
Contributor

chirayudesai commented Jun 20, 2016

Hi,

As you are already aware, Debian has packaged apktool, and for that we've needed to maintain a few patches to this code. Some of there are for debian-specific behaviour, however some could potentially be upstreamed.
The patches can be viewed at debian git

From those, framework_install_path.patch is the first patch I'd like to submit.
Could also potentially discuss and submit use_system_framework.patch, and use_system_aapt.patch later. There have been some suggestions submitted, such as https://bugs.debian.org/827646

Talking about framework_install_path.patch, I pushed it to my fork for easy viewing.
I'm not familiar with OSX, however from a quick search it does seem that there might need to be a different patch. Also, I guess you'll probably want some migration mechanism too for existing users.

Putting this out here for discussion first before submitting any pull requests as I don't feel that any of the patches could be submitted as-is.

Regards,
Chirayu Desai

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Jun 20, 2016

Owner

framework_install_path - I agree, cluttering $HOME is not a good practice. The problem initially was a cross platform solution, but now we can easily specify a location for each major system. It appears both unix/mac can benefit from $HOME/.local/share/apktool.

Windows is a different beast. If my research is correct, then %USERPROFILE%\AppData\Local\Packages\apktool might be the best location.

A migration can be provided via scripts to run during an upgrade, nothing that has to be baked into the tool directly as these frameworks can be remade at any time. If a user doesn't run these scripts, they will have abandoned files. I don't feel comfortable deleting files/directories just in case a user used the folder for storing frameworks.

use_system_framework doesn't look like something we could upstream, as that is just copying the internal framework file to a permanent location to prevent the need of having to extract it from JAR into a location. Let me know if I'm incorrect on this assumption.

use_system_aapt I'm a bit iffy on. Apktool is constantly adding more patches to the host system aapt tool - https://github.com/iBotPeaches/platform_frameworks_base/commits/apktool-mm-2 (recent commits)

These commits allow application building to work with the constant adapting and changing apk structure. While the urge is just use whatever the latest is from Android, I can't confirm success with building using the newest aapt with all applications.

Owner

iBotPeaches commented Jun 20, 2016

framework_install_path - I agree, cluttering $HOME is not a good practice. The problem initially was a cross platform solution, but now we can easily specify a location for each major system. It appears both unix/mac can benefit from $HOME/.local/share/apktool.

Windows is a different beast. If my research is correct, then %USERPROFILE%\AppData\Local\Packages\apktool might be the best location.

A migration can be provided via scripts to run during an upgrade, nothing that has to be baked into the tool directly as these frameworks can be remade at any time. If a user doesn't run these scripts, they will have abandoned files. I don't feel comfortable deleting files/directories just in case a user used the folder for storing frameworks.

use_system_framework doesn't look like something we could upstream, as that is just copying the internal framework file to a permanent location to prevent the need of having to extract it from JAR into a location. Let me know if I'm incorrect on this assumption.

use_system_aapt I'm a bit iffy on. Apktool is constantly adding more patches to the host system aapt tool - https://github.com/iBotPeaches/platform_frameworks_base/commits/apktool-mm-2 (recent commits)

These commits allow application building to work with the constant adapting and changing apk structure. While the urge is just use whatever the latest is from Android, I can't confirm success with building using the newest aapt with all applications.

@chirayudesai

This comment has been minimized.

Show comment
Hide comment
@chirayudesai

chirayudesai Jun 20, 2016

Contributor

On Mon, Jun 20, 2016 at 10:09 PM Connor Tumbleson notifications@github.com
wrote:

framework_install_path - I agree, cluttering $HOME is not a good
practice. The problem initially was a cross platform solution, but now we
can easily specify a location for each major system. It appears both
unix/mac can benefit from $HOME/.local/share/apktool.

I wasn't sure about mac and $HOME/.local/share, but even then, anything
that doesn't create a non-hidden top-level directory in $HOME would be
better.

Windows is a different beast. If my research is correct, then
%USERPROFILE%\AppData\Local\Packages\apktool might be the best location.

Since java's "user.home" already expands to %UserProfile%, we could add an
additional check below the OSX check.
And also move the ".local/share" addition to "user.home" after the windows
patch assignment.

A migration can be provided via scripts to run during an upgrade, nothing
that has to be baked into the tool directly as these frameworks can be
remade at any time. If a user doesn't run these scripts, they will have
abandoned files. I don't feel comfortable deleting files/directories just
in case a user used the folder for storing frameworks.

Right, would also be good for users, they could get rid of old / unused
frameworks.
And also get the latest framework, since the update framework work hasn't
been merged yet.
I agree with there not being any deletion.

use_system_framework doesn't look like something we could upstream, as
that is just copying the internal framework file to a permanent location to
prevent the need of having to extract it from JAR into a location. Let me
know if I'm incorrect on this assumption.

Yes, since we needed a separate framework-res package for usage with
apktool, it was decided to make apktool use that directly rather than
include it in the jar.

use_system_aapt I'm a bit iffy on. Apktool is constantly adding more
patches to the host system aapt tool -
https://github.com/iBotPeaches/platform_frameworks_base/commits/apktool-mm-2
(recent commits)

These commits allow application building to work with the constant
adapting and changing apk structure. While the urge is just use whatever
the latest is from Android, I can't confirm success with building using the
newest aapt with all applications.

I had not noticed that, and I do agree with you here.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1277 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA6G_veXMjaU_gvaHZfHf7ImfyuGmOJgks5qNsIngaJpZM4I53Y0
.

Contributor

chirayudesai commented Jun 20, 2016

On Mon, Jun 20, 2016 at 10:09 PM Connor Tumbleson notifications@github.com
wrote:

framework_install_path - I agree, cluttering $HOME is not a good
practice. The problem initially was a cross platform solution, but now we
can easily specify a location for each major system. It appears both
unix/mac can benefit from $HOME/.local/share/apktool.

I wasn't sure about mac and $HOME/.local/share, but even then, anything
that doesn't create a non-hidden top-level directory in $HOME would be
better.

Windows is a different beast. If my research is correct, then
%USERPROFILE%\AppData\Local\Packages\apktool might be the best location.

Since java's "user.home" already expands to %UserProfile%, we could add an
additional check below the OSX check.
And also move the ".local/share" addition to "user.home" after the windows
patch assignment.

A migration can be provided via scripts to run during an upgrade, nothing
that has to be baked into the tool directly as these frameworks can be
remade at any time. If a user doesn't run these scripts, they will have
abandoned files. I don't feel comfortable deleting files/directories just
in case a user used the folder for storing frameworks.

Right, would also be good for users, they could get rid of old / unused
frameworks.
And also get the latest framework, since the update framework work hasn't
been merged yet.
I agree with there not being any deletion.

use_system_framework doesn't look like something we could upstream, as
that is just copying the internal framework file to a permanent location to
prevent the need of having to extract it from JAR into a location. Let me
know if I'm incorrect on this assumption.

Yes, since we needed a separate framework-res package for usage with
apktool, it was decided to make apktool use that directly rather than
include it in the jar.

use_system_aapt I'm a bit iffy on. Apktool is constantly adding more
patches to the host system aapt tool -
https://github.com/iBotPeaches/platform_frameworks_base/commits/apktool-mm-2
(recent commits)

These commits allow application building to work with the constant
adapting and changing apk structure. While the urge is just use whatever
the latest is from Android, I can't confirm success with building using the
newest aapt with all applications.

I had not noticed that, and I do agree with you here.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1277 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA6G_veXMjaU_gvaHZfHf7ImfyuGmOJgks5qNsIngaJpZM4I53Y0
.

iBotPeaches added a commit that referenced this issue Jun 21, 2016

move default framework location on windows/unix
 - unix - $HOME/.local/share/apktool
 - windows - $HOME/AppData/Local/apktool
 - #1277

iBotPeaches added a commit that referenced this issue Jun 21, 2016

added notes about default framework move
 - #1277
 - added FAQ
 - added migration instructions
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Jun 21, 2016

Owner

Okay, directories are now as follows

  • unix - $HOME/.local/share/apktool
  • windows - $HOME/AppData/Local/apktool
  • mac - $HOME/Library/apktool
Owner

iBotPeaches commented Jun 21, 2016

Okay, directories are now as follows

  • unix - $HOME/.local/share/apktool
  • windows - $HOME/AppData/Local/apktool
  • mac - $HOME/Library/apktool

iBotPeaches added a commit that referenced this issue Aug 6, 2016

force encoding to UTF8
 - pulled from #1277
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Aug 6, 2016

Owner

I took a peek at the patches again and plucked one more (Setting UTF8 for the java file encoding). The change mentioned above is set for the 2.2.0 release, so closing this as I don't believe any of the other patches can be forwarded.

Let me know if you encounter anything else.

Owner

iBotPeaches commented Aug 6, 2016

I took a peek at the patches again and plucked one more (Setting UTF8 for the java file encoding). The change mentioned above is set for the 2.2.0 release, so closing this as I don't believe any of the other patches can be forwarded.

Let me know if you encounter anything else.

@iBotPeaches iBotPeaches closed this Aug 6, 2016

@iBotPeaches iBotPeaches added this to the 2.2.0 milestone Aug 6, 2016

iBotPeaches added a commit that referenced this issue Aug 6, 2016

@chirayudesai

This comment has been minimized.

Show comment
Hide comment
@chirayudesai

chirayudesai Aug 9, 2016

Contributor

Thanks for the updates!
We've imported 2.2.0 to debian, and were able to drop one patch, and remove one line from another. Less code to worry about when doing upstream updates :)
https://anonscm.debian.org/cgit/android-tools/apktool.git/log/?h=debian/2.2.0%2bdfsg-1

Contributor

chirayudesai commented Aug 9, 2016

Thanks for the updates!
We've imported 2.2.0 to debian, and were able to drop one patch, and remove one line from another. Less code to worry about when doing upstream updates :)
https://anonscm.debian.org/cgit/android-tools/apktool.git/log/?h=debian/2.2.0%2bdfsg-1

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