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

Pipeline compiler should not alter specified image #1005

Merged
merged 4 commits into from
Aug 26, 2022

Conversation

6543
Copy link
Member

@6543 6543 commented Jul 2, 2022

address #1003

tested :)

@6543 6543 added bug Something isn't working server wip labels Jul 2, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2022

Codecov Report

Merging #1005 (faf4153) into master (f21d854) will decrease coverage by 0.07%.
The diff coverage is n/a.

❗ Current head faf4153 differs from pull request most recent head 8acb5f6. Consider uploading reports for the commit 8acb5f6 to get more accurate results

@@            Coverage Diff             @@
##           master    #1005      +/-   ##
==========================================
- Coverage   50.47%   50.40%   -0.08%     
==========================================
  Files          83       83              
  Lines        6363     6363              
==========================================
- Hits         3212     3207       -5     
- Misses       2965     2970       +5     
  Partials      186      186              
Impacted Files Coverage Δ
server/logging/log.go 48.83% <0.00%> (-5.82%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@anbraten anbraten changed the title WIP: Pipeline compiler should not alter specifyed image WIP: Pipeline compiler should not alter specified image Jul 30, 2022
@6543 6543 removed the wip label Aug 25, 2022
@6543 6543 added this to the 1.0.0 milestone Aug 25, 2022
@6543 6543 added the refactor delete or replace old code label Aug 25, 2022
@6543
Copy link
Member Author

6543 commented Aug 25, 2022

let's wait ~1-2days and if no errors get reported it's fine

Copy link
Contributor

@thestr4ng3r thestr4ng3r left a comment

Choose a reason for hiding this comment

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

You will still need to remove the workaround here, which was added for the old behavior:

diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go
index e1215a00..cc636fb9 100644
--- a/pipeline/backend/local/local.go
+++ b/pipeline/backend/local/local.go
@@ -62,7 +62,7 @@ func (e *local) Exec(ctx context.Context, proc *types.Step) error {
                Command = append(Command, "plugin-git")
        } else {
                // Use "image name" as run command
-               Command = append(Command, proc.Image[18:len(proc.Image)-7])
+               Command = append(Command, proc.Image)
                Command = append(Command, "-c")

                // Decode script and delete initial lines

Otherwise this happens:

panic: runtime error: slice bounds out of range [18:2]

goroutine 44 [running]:
github.com/woodpecker-ci/woodpecker/pipeline/backend/local.(*local).Exec(0x1400026b920, {0x1011d0758?, 0x14000596940}, 0x140005dcd20)
	/Users/florian/dev/woodpecker/pipeline/backend/local/local.go:65 +0x8a0
github.com/woodpecker-ci/woodpecker/pipeline.(*Runtime).exec(0x1400052c070, 0x140005dcd20)
	/Users/florian/dev/woodpecker/pipeline/pipeline.go:216 +0x140
github.com/woodpecker-ci/woodpecker/pipeline.(*Runtime).execAll.func1()
	/Users/florian/dev/woodpecker/pipeline/pipeline.go:181 +0x1b8
golang.org/x/sync/errgroup.(*Group).Go.func1()
	/Users/florian/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:75 +0x5c
created by golang.org/x/sync/errgroup.(*Group).Go
	/Users/florian/go/pkg/mod/golang.org/x/[email protected]/errgroup/errgroup.go:72 +0xa4

If the slicing is removed, the changes work for my use-cases.

@6543 6543 marked this pull request as ready for review August 25, 2022 18:30
@6543 6543 changed the title WIP: Pipeline compiler should not alter specified image Pipeline compiler should not alter specified image Aug 25, 2022
@6543 6543 requested a review from a team August 26, 2022 12:58
@6543 6543 merged commit 9a57602 into woodpecker-ci:master Aug 26, 2022
@6543 6543 deleted the address-1003 branch August 26, 2022 18:00
@thestr4ng3r
Copy link
Contributor

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor delete or replace old code server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants