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.

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

easy script for self building #74

Closed
wants to merge 1 commit into from

Conversation

elichai
Copy link

@elichai elichai commented Mar 23, 2020

Hi,
It's probably not a priority for you, but still an issue for security conscious people is that I want to know that the code in google-play is the same as in the code I've reviewed.
The ideal solution would be reproducible deterministic builds, and a way to easily compare the hashes.

But for now self compiling is also a great option. so this scripts abstract out all the weird complexities android sdk has and all the environment variables.
and it also provides a persistent self generated keystore.

@MiriShalev
Copy link
Contributor

Thanks @elichai ! This would be an awesome addition when we begin our CI efforts!

@elichai
Copy link
Author

elichai commented Mar 23, 2020

I can write a Travis script just to test building in Android if it helps :)

@@ -0,0 +1,38 @@
FROM ubuntu:18.04

Choose a reason for hiding this comment

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

why not use alpine and apk?

Copy link
Author

Choose a reason for hiding this comment

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

yeah, this was prototyping to allow myself to compile the apk so I won't need to install from the google play and I could skip one trust hop.

I didn't put too much effort into improving this afterwards because I wasn't sure if you guys are even interested. if you are I can do that :)

Copy link

@Wassap124 Wassap124 Mar 26, 2020

Choose a reason for hiding this comment

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

not part of the team 🙌 so I can't really approve in their name.
Just wanted to start contributing by helping with open PRs

Choose a reason for hiding this comment

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

regardless, alpine is 10 times lighter than ubuntu which is pretty optimal for creating containers

IMAGE_TAG_FILE="image.tag"
ALIAS="magen"

GUES_PATH=/magen/hamagen-react-native

Choose a reason for hiding this comment

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

what does GUES stand for?

Copy link
Author

Choose a reason for hiding this comment

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

typo from GUEST :)

Choose a reason for hiding this comment

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

Notify me once you push and I'll resolve

@codecov
Copy link

codecov bot commented Apr 1, 2020

Codecov Report

Merging #74 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master     #74   +/-   ##
======================================
  Coverage    8.69%   8.69%           
======================================
  Files          57      57           
  Lines        1887    1887           
  Branches      376     376           
======================================
  Hits          164     164           
  Misses       1576    1576           
  Partials      147     147           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 158a5d3...880c5c4. Read the comment docs.

@emanuelb
Copy link

emanuelb commented Jul 29, 2020

This script has many problems:

  • install both curl/wget (only 1 is needed to download files...)
  • use ubuntu:18 instead of devel (also better to use debian:sid for this)
  • install nodejs not through package-manager
  • install command-line-tools without verification (no need to do this download manually at all, gradle will handle it automatically, or use android-sdk package instead)
  • manual download of platform-tools (no need if gradle used for this which will handle it automatically)
  • many not merged RUN commands (many not-needed layers created)
  • npm install in development mode (not production which should be enough for apk building)
  • git clone inside the container (better to use COPY instructions from already downloaded source-code, manage git operations outside)

I think it should be closed, as there is issue #243 to cover for it.

@elichai
Copy link
Author

elichai commented Jul 29, 2020

Yep, a lot has changed since this script was written.
it was mostly written as docs to what need to be installed to allow people to compile themselves.

@elichai elichai closed this Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants