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

scripts/BuildScripts/BuildCommon.mk: unique names for development images #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

austin987
Copy link
Contributor

For your consideration.

I often have a few branches in play, and end up manually renaming images when testing something. Got tired of that, came up with this, figured I'd upstream it. If not interested, no worries, I'll maintain it locally.

@SolidHal
Copy link
Owner

SolidHal commented Dec 2, 2020

I think this is a great improvement! definitely handy.
FYI it breaks ci since the images end up as different names, I'll take care of it after the merge though.

@SolidHal
Copy link
Owner

SolidHal commented Dec 2, 2020

hmm, maybe CI doesn't end up with a branch name? This is the image name it ends up with:
PrawnOS-Shiba-armhf-git--3870b05651d1aac4b9a05fd23fc8b8aff30cf1b0.img
that double - indicates to me PRAWNOS_GIT_BRANCH is empty

@SolidHal
Copy link
Owner

SolidHal commented Dec 2, 2020

Seems github CI doesn't use a branch, and instead just has a checkout at the sha.
Could you add a case to catch this situation? maybe if the PRAWNOS_GIT_BRANCH variable is empty, set it as NOBRANCH? I'm open to other suggestions here.

Second request: could you modify build-image.sh and the github workflows to match this new naming scheme?

Final thought: What happens if the image is built on a sha but there are a bunch of uncommitted changes? Could we indicate somehow that its a modified version of that sha? This isn't required at all, more just an idea.

Thanks again for this. I think this change makes a lot of sense overall. :)

@austin987
Copy link
Contributor Author

Sure, I'll take a look.

I ignored the CI failures since it's been broken so much lately (I know I originally added it, and I do care, but as you pointed out before, it's not currently reliable, so I didn't consider it a blocker).

At some point I may look at moving to a container based solution, but I have other things I'd like to see done first (and when I tried in containers, things didn't work so well either for me, maybe it's improved since then).

@SolidHal
Copy link
Owner

SolidHal commented Dec 3, 2020

I actually fixed the CI a week or so ago, so it should be reliable again :)

@austin987
Copy link
Contributor Author

Note: as they're semi-related, continuing discussion on #250.

@SolidHal SolidHal self-requested a review July 26, 2022 05:00
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