depends_on: [] doesn't enable DAG mode

[KamilaBorowska]

Component

server

Describe the bug

depends_on: [] like specified in https://github.com/woodpecker-ci/woodpecker/blob/c1a1f7c10b3c0840bad9c0b25fcd7f5ff8b0173b/.woodpecker/securityscan.yml is useless, and doesn't enable DAG mode as documented on https://woodpecker-ci.org/docs/usage/workflow-syntax#depends_on.

This is because DAG is only enabled for non-empty lists:

woodpecker/pipeline/frontend/yaml/compiler/dag.go

Lines 44 to 51 in c1a1f7c

	func (c dagCompiler) isDAG() bool {
	for _, v := range c.steps {
	if len(v.dependsOn) != 0 {
	return true
	}
	}
	return false
	}

System Info

{"source":"https://github.com/woodpecker-ci/woodpecker","version":"2.1.1"}

Additional context

No response

Validations

• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Checked that the bug isn't fixed in the next version already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]
• Check that this is a concrete bug. For Q&A join our Discord Chat Server or the Matrix room.

[anbraten]

Should check for nil, but afaik we will run into depends_on: [] being nil as well when parsed by the yaml parser: go-yaml/yaml#42

[6543]

@anbraten we chould allow depends_on: [""] ...

as this would also address #3047

[6543]

if we merge #3008 that should be addressed

[KamilaBorowska]

I don't think #3008 is really a solution. depends_on: [] still won't work, and depends_on: [""] looks like some kind of workaround that shouldn't be necessary.

[KamilaBorowska]

As for go-yaml/yaml#42, as far as I can tell, parsing empty arrays works as expected, the following Go program:

package main

import (
	"fmt"

	"gopkg.in/yaml.v3"
)

type Item struct {
	DependsOn []string `yaml:"depends_on"`
}

func main() {
	var a Item
	yaml.Unmarshal([]byte("{}"), &a)
	fmt.Println(a.DependsOn == nil)
	out, err := yaml.Marshal(a)
	if err != nil {
		panic(err)
	}
	fmt.Println(string(out))

	var b Item
	yaml.Unmarshal([]byte("depends_on: []"), &b)
	fmt.Println(b.DependsOn == nil)
	out, err = yaml.Marshal(b)
	if err != nil {
		panic(err)
	}
	fmt.Println(string(out))
}

Outputs this:

true
depends_on: []

false
depends_on: []

An empty array is not null, unlike an unprovided array. However, it's worth noting that that when marshaled with YAML, nil array ends up being [].

However, there is a workaround to make YAML marshaling work as well (not just unmarshaling) of providing the type as *[]string. Then, both marshaling and unmarshaling will work fine.

package main

import (
	"fmt"

	"gopkg.in/yaml.v3"
)

type Item struct {
	DependsOn *[]string `yaml:"depends_on,omitempty"`
}

func main() {
	var a Item
	yaml.Unmarshal([]byte("{}"), &a)
	fmt.Println(a.DependsOn == nil)
	out, err := yaml.Marshal(a)
	if err != nil {
		panic(err)
	}
	fmt.Println(string(out))

	var b Item
	yaml.Unmarshal([]byte("depends_on: []"), &b)
	fmt.Println(b.DependsOn == nil)
	out, err = yaml.Marshal(b)
	if err != nil {
		panic(err)
	}
	fmt.Println(string(out))
}

This will output:

true
{}

false
depends_on: []

This works fine with *StringOrSlice type as well.

[6543]

could you create a pull :)

[qwerty287]

I agree with @lafriks and don't think an empty depends_on should be used to enable this, so I prefer a separate solution like #3047.
My main issues are, that having empty slices looks very weird and it is very unintuitive. If I'd just look at a config, having depends_on: [] set somewhere, I wouldn't expect that this means that it will run in parallel. Having a separate key like order suggested in #3047 is pretty clear.