toolshed/abra
toolshed/abra
4
2
Fork 18
Code Issues 27 Pull Requests 3 Packages Releases 40 Activity

fix: make branches available as recipe source #453

Closed
stevensting wants to merge 3 commits from branch-recipes into main
pull from: branch-recipes
merge into: toolshed:main
Conversation 7 Commits 3 Files Changed 3 +16 -24
stevensting commented 2024-12-26 17:10:50 +00:00
First-time contributor
Copy Link
No description provided.
🎉 1
stevensting added the
enhancement
 label 2024-12-26 17:10:50 +00:00
stevensting self-assigned this 2024-12-26 17:10:50 +00:00
stevensting added 2 commits 2024-12-26 17:10:51 +00:00
allow to define remote branches as recipe source
Some checks failed
continuous-integration/drone/push Build is failing
Details
161f2127d6
cloned repos shall contain all branches
Some checks failed
continuous-integration/drone/push Build is failing
Details
continuous-integration/drone/pr Build is failing
Details
776a8414d4
stevensting requested review from decentral1se 2024-12-26 17:11:16 +00:00
stevensting requested review from p4u1 2024-12-26 17:11:16 +00:00
decentral1se reviewed 2024-12-26 18:51:32 +00:00
decentral1se left a comment
Owner
Copy Link

Oh this is really cool @stevensting! Thanks for working on this!

A few minor comments below.

I'd be quite nervous to merge this without a test. Got any ideas? If it's really too awkward, lemme know how you manually tested it and I can also try.

Oh this is really cool @stevensting! Thanks for working on this! A few minor comments below. I'd be quite nervous to merge this without a test. Got any ideas? If it's really too awkward, lemme know how you manually tested it and I can also try.
pkg/recipe/compose.go Outdated
@ -95,6 +95,7 @@ func (r Recipe) GetVersionLabelLocal() (string, error) {
for _, service := range config.Services {
for label, value := range service.Deploy.Labels {
log.Debugf("checking deploy label '%s'", label)
decentral1se commented 2024-12-26 18:49:15 +00:00
Owner
Copy Link

Could we make this message more specific, e.g. what are we checking for?

Also, you don't log the value, is that by choice?

Could we make this message more specific, e.g. what are we checking for? Also, you don't log the `value`, is that by choice?
decentral1se marked this conversation as resolved
pkg/recipe/git.go Outdated
@ -68,6 +68,7 @@ func (r Recipe) EnsureExists() error {
// EnsureVersion checks whether a specific version exists for a recipe.
func (r Recipe) EnsureVersion(version string) (bool, error) {
isChaosCommit := false
log.Debugf("Ensuring version '%s'", version)
decentral1se commented 2024-12-26 18:49:48 +00:00
Owner
Copy Link

%s/Ensuring/ensuring (lowercase convention everywhere else in the codebase).

Also, can you make this more specific, e.g. using the r.Name there too?

`%s/Ensuring/ensuring` (lowercase convention everywhere else in the codebase). Also, can you make this more specific, e.g. using the `r.Name` there too?
decentral1se marked this conversation as resolved
decentral1se commented 2024-12-30 14:32:23 +00:00
Owner
Copy Link

@stevensting FYI there is some prior art in unit testing this stuff below, thanks to @ammaratef45 🎉 Let me know if you have spoons / cycles to finish this up in the next days, we can try to coordinate to get this change into the new release? Thanks!

74108b0dd9/pkg/git/init_test.go (L11-L35)

@stevensting FYI there is some prior art in unit testing this stuff below, thanks to @ammaratef45 🎉 Let me know if you have spoons / cycles to finish this up in the next days, we can try to coordinate to get this change into the new release? Thanks! https://git.coopcloud.tech/coop-cloud/abra/src/commit/74108b0dd91c8c0322e821c3aeaf6b3e5f704e37/pkg/git/init_test.go#L11-L35
stevensting added 1 commit 2025-01-03 14:26:17 +00:00
fix review findings
Some checks failed
continuous-integration/drone/pr Build is failing
Details
continuous-integration/drone/push Build is failing
Details
65c9f4c48a
decentral1se commented 2025-01-03 19:29:25 +00:00
Owner
Copy Link

(Copying over from chats FYI)

i tested, it doesn't seem to work... i was reading #249 and i don't understand why SingleBranch: false doesn't work in our case? for example, with your changes, if you rebase on latest main and run abra catalogue generate gitea (master) // abra catalogue generate collabora (main) - you get an error for collabora that it can't find the ref?

(Copying over from chats FYI) > i tested, it doesn't seem to work... i was reading [`#249`](https://github.com/go-git/go-git/issues/249) and i don't understand why SingleBranch: false doesn't work in our case? for example, with your changes, if you rebase on latest main and run `abra catalogue generate gitea` (`master`) // `abra catalogue generate collabora` (`main`) - you get an error for collabora that it can't find the ref?
decentral1se changed title from Make branches available as recipe source to fix: make branches available as recipe source 2025-01-03 19:32:01 +00:00
decentral1se commented 2025-01-03 19:44:08 +00:00
Owner
Copy Link

Another terrifying reference: https://github.com/go-git/go-git/issues/363#issuecomment-943534705 Even Git itself has no idea how to figure this out 😱

There's an implementation of "if transport supports directly peeking where HEAD points to, use that" here which I also tested but refs["HEADS"].Target() returns nil and it explodes for the Collabora recipe (uses main).

We're definitely not doing "if the git_default_branch_name (from the git configuration: key init.defaultbranch) exists at remote, use it" either.

We could try to implement all that?

@stevensting maybe you can explain what others branches you're using that are not master/main and how you've configured your Git setup? Then we can know how to "guess" this correctly in our implementation.

🤔🤔🤔

Another terrifying reference: https://github.com/go-git/go-git/issues/363#issuecomment-943534705 Even Git itself has no idea how to figure this out 😱 There's an implementation of "if transport supports directly peeking where HEAD points to, use that" [here](https://github.com/vinted/sbomsftw/pull/102/files) which I also tested but `refs["HEADS"].Target()` returns `nil` and it explodes for the Collabora recipe (uses `main`). We're definitely not doing "if the git_default_branch_name (from the git configuration: key init.defaultbranch) exists at remote, use it" either. We could try to implement all that? @stevensting maybe you can explain what others branches you're using that are not `master`/`main` and how you've configured your Git setup? Then we can know how to "guess" this correctly in our implementation. 🤔🤔🤔
decentral1se added this to the abra v0.10.0 project 2025-01-04 09:27:24 +00:00
decentral1se commented 2025-01-04 12:42:17 +00:00
Owner
Copy Link

Due to this being a hornest nest, we're gonna abandon it (just chatted on Matrix). I'm gonna try fix #464 which I hope is unrelated. If that works out, then I guess we will have to say "this is an upstream limitation and please use main for branches". Updates coming soon hopefully.

Due to this being a hornest nest, we're gonna abandon it (just chatted on Matrix). I'm gonna try fix https://git.coopcloud.tech/toolshed/abra/issues/464 which I hope is unrelated. If that works out, then I guess we will have to say "this is an upstream limitation and please use main for branches". Updates coming soon hopefully.
decentral1se closed this pull request 2025-01-04 12:42:17 +00:00
p4u1 commented 2025-01-04 13:01:12 +00:00
Member
Copy Link

The goal of this PR was to make RECIPE=backupbot-two:mycoolbranch work. This should be possible right?

The goal of this PR was to make `RECIPE=backupbot-two:mycoolbranch` work. This should be possible right?
decentral1se commented 2025-01-04 13:46:28 +00:00
Owner
Copy Link

The goal of this PR was to make RECIPE=backupbot-two:mycoolbranch work. This should be possible right?

#468

> The goal of this PR was to make RECIPE=backupbot-two:mycoolbranch work. This should be possible right? https://git.coopcloud.tech/toolshed/abra/issues/468
👍 1
decentral1se moved this to Done in abra v0.10.0 on 2025-04-14 22:00:59 +00:00
decentral1se moved this to Done in abra v0.10.0 on 2025-04-14 22:01:01 +00:00
decentral1se moved this to Done in abra v0.10.0 on 2025-04-16 05:16:08 +00:00
decentral1se moved this to Done in abra v0.10.0 on 2025-04-16 05:16:10 +00:00
decentral1se moved this to Done in abra v0.10.0 on 2025-04-19 07:28:29 +00:00
decentral1se moved this to Done in abra v0.10.0 on 2025-04-21 17:48:16 +00:00
Some checks failed
continuous-integration/drone/pr Build is failing
Details
continuous-integration/drone/push Build is failing
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
decentral1se
p4u1
Labels
Clear labels
bug
Something is not working
build
go build related issues
ci/cd
Building things with CI/CD
contributing
Dealing with contributing
critical fix
https://docs.coopcloud.tech/federation/resolutions/passed/010/
design
UI/UX
documentation
Documenting all the things
duplicate
This issue or pull request already exists
enhancement
New feature
help wanted
Need some help
invalid
Something is wrong
meta
relating to the project or repo itself
question
More information is needed
release
Release management
release-candidate
Related to the new release candidate
security
Security related
test
Unit/integration testing
wontfix
This won't be fixed
No Label
enhancement
Milestone
No items
No Milestone
Projects
Clear projects
No project abra v0.10.0
Assignees
Clear assignees
3wordchant abra-bot ammaratef45 Apfelwurm carla cas decentral1se fauno kawaiipunk knoflook lambdabundesverband moritz p4u1 simon trav val yksflip
No Assignees stevensting
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#453
No description provided.
Delete Branch "branch-recipes"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?