Skip to content

get rid of the case statements because it needs maintenance #755

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

Closed

Conversation

strayedelectron
Copy link
Contributor

In order to get rid of the long list of "supported" lineage branches I propose a solution that does not need any more maintenance at each lineage version as long as the offset between lineage version and android version remains 7.
Background is, that I often could not build the newest lineage development version because of the lineage version in the case statements was outdated.

Added some predicted LOS and Android versions, so that future compiling is not hindered
use lineage numbers to split between legacy and new build instead of maintaining a list of branches
select branch and android version without maintaining a list
fix space between variable and equal sign
fix space between variable and equal sign
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to do any validation of $branch. We currently display an error if a user specifies a branch that is not supported. I don't want to lost that. Maybe we could check against a list of supported branches (e.g. the branches listed at https://github.com/LineageOS/android/branches/all).

@petefoth
Copy link
Contributor

petefoth commented Apr 9, 2025

Thinking some more about this, I think we should do the following:

  1. Create a new file supported_branches.txt, content being the branched listed at https://github.com/LineageOS/android/branches/all
  2. in init.sh, search supported_branches.txt for $branch:
    • if no match is found , exit with error message
    • if a match is found, continue with the processing in these changes to
      • call the correct build script from init.sh
      • setup themuppets_branch and android_version

These changes would need to be implemented and tested before merging into the master branch. Tests should involve calling docker run with a selection of valid and invalid values of BRANCH_NAME to check the required behaviour. (This may need inserting some logging to display $themuppets_branch and $android_version)

@strayedelectron
Copy link
Contributor Author

strayedelectron commented Apr 10, 2025

Would the supported_branches.txt file be inside this git repository? Or would it be generated each time the init.sh script is called?
I would vote against a file inside this git, because it will result in a delay until someone pushes an update. In the meantime compiling would be impossible because of some list ist not up to date.

We currently display an error if a user specifies a branch that is not supported. I don't want to lost that.

Why? What is the drawback if a user decides to compile the newest beta branch that is just now available?
At least in my experience I had this situation now 4-5 times that I could not compile because that "not supported" message. Each time I hat two options: One to wait until this repo gets an update. Two just hack the next branch version in that file myself.

The only reason I see for a branch dependent code is for example the switch between legacy and non-legacy build.

@petefoth
Copy link
Contributor

Would the supported_branches.txt file be inside this git repository?

Yes

I would vote against a file inside this git, because it will result in a delay until someone pushes an update. In the meantime compiling would be impossible because of some list ist not up to date.

It will be trivial to add the new branch in the file, and would likely be done more promptly because it is more straightforward than the current code

@petefoth
Copy link
Contributor

petefoth commented Apr 10, 2025

I'll give this approach a try (and some testing) if you can fix the shellcheck errors

./src/new_build.sh:162:45: note: Double quote to prevent globbing and word splitting. [SC2086]

and

./src/init.sh:69:12: note: Possible misspelling: BRANCH_NAME may not be assigned. Did you mean BRANCH_NUM? [SC2153]

(I can't fix them because I don't have write access to your repo)

Replaced the part that may cause a globbing issue
Remove useless BRANCH_NUM variable.
@petefoth
Copy link
Contributor

Thanks. These changes are now in my test branch, and I hope to get them tested in the next few days when I get home from holiday

@petefoth
Copy link
Contributor

Thanks for this PR. I'll close it since the code was merged into the master branch in #757

@petefoth petefoth closed this Apr 15, 2025
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.

2 participants