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.

Sign up for GitHub

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

Jump to bottom

Fix for sdk25 #2

Merged
merged 9 commits into from
Mar 4, 2017
Merged

Fix for sdk25 #2

merged 9 commits into from
Mar 4, 2017

Conversation

rjaros87
Copy link
Owner

@rjaros87 rjaros87 commented Mar 1, 2017

No description provided.

@rjaros87
Copy link
Owner Author

rjaros87 commented Mar 1, 2017

@Brantone , @gildegoma please review. Fix for gildegoma#34

@rjaros87 rjaros87 force-pushed the windows_fix_sdk25 branch 3 times, most recently from a54c2e3 to b2df530 Compare March 1, 2017 12:56
@Brantone
Copy link

Brantone commented Mar 1, 2017

Seems like lots of other peripheral changes (ex: remove tailor from Rakefile), and bunch of white space changes, which make it somewhat cumbersome to isolate actual changes to SDK25.
Possible to split out the actual changes for gildegoma#34 SDK25 and extras? Or even maybe few extra comments for other un-referenced chanages?

Brantone
Brantone reviewed Mar 1, 2017
View reviewed changes
Rakefile

desc 'Lint Ruby code'
task :tailor do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removal?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubocop checks the the same and even more.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and also we execute foodcritic ;)

rjaros87
rjaros87 commented Mar 2, 2017
View reviewed changes
.kitchen.yml
jdk_version: 7

- name: default
- name: legacy
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suite to check the backward compatibility

.kitchen.yml
components:
- platform-tools
- build-tools-22.0.1
- android-22
- sys-img-armeabi-v7a-android-22

- name: default
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suite to check the latest SDK and default recipe without modification attributes android-sdk

attributes/default.rb Outdated
default['android-sdk']['checksum']['mac_os_x'] = '593544d4ca7ab162705d0032fb0c0c88e75bd0f42412d09a1e8daa3394681dc6'
default['android-sdk']['checksum']['windows'] = '23d5686ffe489e5a1af95253b153ce9d6f933e5dbabe14c494631234697a0e08'

if ::Gem::Version.new(node['android-sdk']['version']) < ::Gem::Version.new('25')
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We check the SDK version and set node['android-sdk']['legacy_sdk'] attribute which will be used in our recipies. Also there are different urls.

recipes/unix.rb Outdated
@@ -71,6 +73,7 @@
owner node['android-sdk']['owner']
group node['android-sdk']['group']
backup node['android-sdk']['backup_archive']
strip_components node['android-sdk']['legacy_sdk'] ? 1 : 0
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major fix for unix system is just simply extract archive to android-sdk directory without strip directory (by default ark enter to the first directory and then extract the content (default value: 1))

recipes/windows.rb Outdated
::File.rename "#{setup_root}/android-sdk-windows", "#{setup_root}/#{node['android-sdk']['name']}"
if node['android-sdk']['legacy_sdk']
::File.rename "#{setup_root}/android-sdk-windows", "#{setup_root}/#{node['android-sdk']['name']}"
else
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major fix for windows system

@rjaros87
Copy link
Owner Author

rjaros87 commented Mar 2, 2017

Extras are only Rakefile, Gemfile and .rubocop

.kitchen.yml Had a bad indentation and strangely combined attributes. Of course, I had to add tests for SDK 25 and SDK 24

.travis.yml I had problem with Ubuntu machine, because they don't see package urls 404 - apt-get update solves the problem and by the way I raised versions of ruby to be the same as is in the ChefDK

Berksfile (test-kitchen) the same problems as was on Travis

default.rb and recipies - see my comments above

test/integration/* - because now we have two suites (default and legacy) I used shared context to not duplicate scenario :)

dna.json - just update sdk version

Hope my comments will help.

@gildegoma
Copy link

gildegoma commented Mar 2, 2017 •
edited
Loading

Thank you a lot and again @rjaros87 and @Brantone to collaborate on this issue.
I'd like to precise that gildegoma#34 is a bug fix issue for the 0.2.x version of the cookbook, and its fix must absolutely be focused and be based on cb67ebd (0.2.0 without any additional changes). The backwards compatible approach of auto-detected node['android-sdk']['legacy_sdk'] looks great at first glance. I'd really love to receive a PR that only address the issue for the Linux platform (instead of being based on gildegoma#33 stuff). The work for version 0.3.0 is a quite larger topic, where more refactorings are fully allowed.

❤️ ❤️ ❤️

@gildegoma gildegoma mentioned this pull request Mar 2, 2017
Closed
@rjaros87 rjaros87 mentioned this pull request Mar 2, 2017
Closed
gildegoma and others added 7 commits March 3, 2017 00:54
@gildegoma
Bump and reorganize FoodCritic
04ac4ea 
In order to prepare a more significant switch to ChefDK, the FoodCritic
exceptions are now defined in .foodcritic, and no longer in the
Rakefile.

Enable back FC041 (download are done via the ark cookbook only)

Tolerate FC053 as the 'recommends' keyword is still worth to be present,
even though Chef tools are not using this meta data.
@rjaros87 @gildegoma
[gildegomaGH-34] Fix broken installation of SDK Tools 25+
5ef234b
@gildegoma
Update CHANGELOG and README according to v0.2.1
23aae55
@gildegoma
[gildegomaGH-34] Always download .zip packages with new URL
586bb82 
After four random checks, it seems that the new HTTPS-based
repository provides all the Linux package history. The archives are in
.zip format, and don't require to strip an `android-sdk-linux` base
directory.

Since the new .zip archives only contain the 'tools' directory, it is
no longer necessary to fix the permissions and ownership for
the 'add-ons' or 'platforms' that were present in the .tgz file.
@gildegoma
v0.2.1 QA: Update Vagrantfile and .kitchen.yml
69ec362
@rjaros87
Merge branch 'master' of github.com:rjaros87/chef-android-sdk into wi…
a27931a 
…ndows_fix_sdk25
@rjaros87
Update test and refactor Unix recipe
e877e10
@rjaros87
Copy link
Owner Author

rjaros87 commented Mar 4, 2017

Merged with upstream master :) and refactor unix recipe

rjaros87
rjaros87 commented Mar 4, 2017
View reviewed changes
recipes/unix.rb
#
# Fix non-friendly 0750 permissions in order to make android-sdk available to all system users
#
%w(add-ons platforms tools).each do |subfolder|
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this anymore :) by default in archive tools has 755

@rjaros87 rjaros87 merged commit 4c8d865 into windows Mar 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Brantone Brantone Brantone left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rjaros87 @Brantone @gildegoma