toolshed/abra
4
4
Fork 23
Code Issues 80 Pull Requests 10 Packages Releases 45 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
Owner
Copy Link
Copy Source
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
continuous-integration/drone/push Build is failing
Details
161f2127d6
cloned repos shall contain all branches
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
Copy Source

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
Copy Source

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
Copy Source

%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
Copy Source

@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
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
Copy Source

(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
Copy Source

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
Copy Source

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
Owner
Copy Link
Copy Source

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
Copy Source

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 are pending
continuous-integration/drone/pr Build is failing
Details
continuous-integration/drone/push Build is failing
Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.
Sign in to join this conversation.
Reviewers
No Reviewers
p4u1
decentral1se
Labels
Clear labels
bug
Something is not working
 build
go build related issues
 ci/cd
Building things with CI/CD
 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
 easy-first-issue
Something for new people to get stuck into. We hope it's easy!
 enhancement
New feature
 help wanted
Need some help
 i10n
Everything to do with localisation
 i18n
Everything to do with internationalisation
 installer
Everything to do with the install script.
 invalid
Something is wrong
 question
More information is needed
 release
Release management
 release-candidate
Related to the new release candidate
 security
Security related
 tech-debt
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 aadil (Aadil Ayub) abra-bot (Abra Bot) ammaratef45 amras (Sarma) Apfelwurm BornDeleuze Brooke carla cas (Cassowary) coopcloud cyrnel decentral1se (d1) dede devydave fauno (fauno) iexos jade (Jade Ambrose) jjsfunhouse jmakdah2 (Jackie Makdah) joe-irving (Joe Irving) kawaiipunk (KawaiiPunk) knoflook kolaente lambdabundesverband linnealovespie (April) moosemower moritz notplants oxaliq (sorrel) p4u1 pharaohgraphy (Andrew 🐦‍🔥❤️‍🔥✴️) renovate-bot (Comrade Renovate Bot) ripclap simon sixsmith (Sixsmith) stevensting trav val (val (he/him)) yksflip
No Assignees stevensting
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: toolshed/abra#453
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?