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

Does not build with the default setup on M1. #4644

Closed
antonmagnus opened this issue Oct 12, 2022 · 2 comments · Fixed by #4675
Closed

Does not build with the default setup on M1. #4644

antonmagnus opened this issue Oct 12, 2022 · 2 comments · Fixed by #4675
Assignees
@adhiamboperes
Labels
good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@antonmagnus
Copy link
Contributor

The project does not build with the default setup.
Running macOS Monterey 12.1 M1 building for pixel3a API 29

Following the setup for mac here
Install oppia-android and Run the app from Android Studio.

`Execution failed for task ':model:generateProto'.

Could not resolve all files for configuration ':model:protobufToolsLocator_protoc'.
Could not find protoc-3.8.0-osx-aarch_64.exe (com.google.protobuf:protoc:3.8.0).
Searched in the following locations:
https://repo.maven.apache.org/maven2/com/google/protobuf/protoc/3.8.0/protoc-3.8.0-osx-aarch_64.exe`

Expected to build without issue.

The issue seems like the same one mentioned here #3912
It was fixed by adding the proto platform to local.properties using the setup script (mentioned here #3943)

In the build Gradle model file the variable is fetched from root project variables and not from the local.properties.
I solved the issue by correcting this. It still remains to be tested on other environments.
@BenHenning
Copy link
Member

Thanks @antonmagnus for the context, and the notes on remediation. This should be straightforward for someone to address.

@BenHenning BenHenning added good first issue This item is good for new contributors to make their pull request. issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged. labels Oct 13, 2022
@antonmagnus
Copy link
Contributor Author

@BenHenning I have a fix now! Just gonna make a PR and have it tested on different OS

@antonmagnus antonmagnus mentioned this issue Oct 13, 2022
Closed
@adhiamboperes adhiamboperes mentioned this issue Oct 27, 2022
Merged
6 tasks
BenHenning added a commit that referenced this issue Oct 31, 2022
@adhiamboperes @BenHenning
Fix #4644: Set custom protobuf artifact for macOS (#4675)
e4e4de6 
## Explanation
Fix #4644: 

> `Execution failed for task ':model:generateProto'.`
> 
> Could not resolve all files for configuration
':model:protobufToolsLocator_protoc'.
> Could not find protoc-3.8.0-osx-aarch_64.exe
(com.google.protobuf:protoc:3.8.0).
> Searched in the following locations:
>
[https://repo.maven.apache.org/maven2/com/google/protobuf/protoc/3.8.0/protoc-3.8.0-osx-aarch_64.exe`](https://repo.maven.apache.org/maven2/com/google/protobuf/protoc/3.8.0/protoc-3.8.0-osx-aarch_64.exe%60)

Changed the conditional check for the `protobuf_platform` property from
checking for property presence to checking for the current os. This is
because `project.rootProject.hasProperty('protobuf_platform')` does not
actually read the properties file as it is meant to, therefore the if
condition was never met and the artifact was never set correctly.

I tested the existing implementaion by adding in build.gradle:
`println project.rootProject.property('protobuf_platform') `
This throws an error: 
`Caused by: groovy.lang.MissingPropertyException: Could not get unknown
property 'protobuf_platform' for root project 'oppia-android' of type
org.gradle.api.Project. `
Which shows that the current solution for reading the property from
l`ocal.properties` file does not work correctly.

This solution changes how the property is read by loading the
local.properties file with inputstream.

## Essential Checklist
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

Co-authored-by: Ben Henning <[email protected]>
Repository owner moved this from Todo to Done in [Team] Developer Workflow & Infrastructure - Android Oct 31, 2022
@adhiamboperes adhiamboperes self-assigned this Nov 1, 2022
@Subhadiptech Subhadiptech mentioned this issue Nov 14, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adhiamboperes adhiamboperes

Labels
good first issue This item is good for new contributors to make their pull request. Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Developer Workflow & Infrastructure - Android
Archived in project
Milestone
No milestone
3 participants
@BenHenning @antonmagnus @adhiamboperes