Fix builder-fetch.sh path

[ianlewis]

Signed-off-by: Ian Lewis ianlewis@google.com

[asraa]

somewhat naive question but could this script have been invoked after changing to the ./BUILDER_CHECKOUT__DIR/ here

slsa-github-generator/.github/actions/generate-builder/action.yml

Line 78 in 55d5ea5

run: ./__BUILDER_CHECKOUT_DIR__/.github/actions/generate-builder/generate-builder.sh

?

(that way the script doesn't need to know the outer context)

[ianlewis]

somewhat naive question but could this script have been invoked after changing to the ./BUILDER_CHECKOUT__DIR/ here

slsa-github-generator/.github/actions/generate-builder/action.yml

Line 78 in 55d5ea5

run: ./__BUILDER_CHECKOUT_DIR__/.github/actions/generate-builder/generate-builder.sh

?
(that way the script doesn't need to know the outer context)

Yeah, I thought about that. We could probably set the working dir.

[asraa]

Yeah, I thought about that. We could probably set the working dir.

I think it would be somewhat clearer, but feel free to push back: we know this one works

[ianlewis]

Yeah, I thought about that. We could probably set the working dir.

I think it would be somewhat clearer, but feel free to push back: we know this one works

I agree. I would like to keep the script context free if possible.

[laurentsimon]

somewhat naive question but could this script have been invoked after changing to the ./BUILDER_CHECKOUT__DIR/ here

slsa-github-generator/.github/actions/generate-builder/action.yml

Line 78 in 55d5ea5

run: ./__BUILDER_CHECKOUT_DIR__/.github/actions/generate-builder/generate-builder.sh

?
(that way the script doesn't need to know the outer context)

Yeah, I thought about that. We could probably set the working dir.

what do you mean by outer context? I think we need to agree on the way all actions invoke scripts for consistency. Otherwise we'll start running into subtle problems: the folders to access the source code (PROJECT_CHECKOUT_DIR) won't be accessible the same way from different scripts, etc. Keeping the invocation from the default director simplifies the code in general, I think
Fyi, Actions don't support working-directory, so setting it will add inconsistency across scripts and Actions well

[asraa]

what do you mean by outer context?

The outer context being the action YAML that's invoking it.

OK thinking about it from that angle, I'd actually assume that scripts in internal actions should be running relative to that action, not the root of the slsa-github-generator repository

[laurentsimon]

what do you mean by outer context?

The outer context being the action YAML that's invoking it.

OK thinking about it from that angle, I'd actually assume that scripts in internal actions should be running relative to that action, not the root of the slsa-github-generator repository

I don't think we can assume that. Sometimes the internal Actions need to access the PROJECT_CHECKOUT_DIR. Having the working directory the top-level root means we can keep the access the same and can easily identify what's invoking: something in the project repo or the builder repo. If we remove this, I'm afraid we're going to start wondering what the working directory is, and make mistakes overwriting things we should not. We want to be sure we don't overwrite builder's code, for example.

[ianlewis]

what do you mean by outer context?

The outer context being the action YAML that's invoking it.

OK thinking about it from that angle, I'd actually assume that scripts in internal actions should be running relative to that action, not the root of the slsa-github-generator repository

what do you mean by outer context?

The outer context being the action YAML that's invoking it.
OK thinking about it from that angle, I'd actually assume that scripts in internal actions should be running relative to that action, not the root of the slsa-github-generator repository

I don't think we can assume that. Sometimes the internal Actions need to access the PROJECT_CHECKOUT_DIR. Having the working directory the top-level root means we can keep the access the same and can easily identify what's invoking: something in the project repo or the builder repo. If we remove this, I'm afraid we're going to start wondering what the working directory is, and make mistakes overwriting things we should not. We want to be sure we don't overwrite builder's code, for example.

Right, Unfortunately a lot of actions need to do something outside of their own directory in the workspace. If there is any real consistency to be had, it's probably, as @laurentsimon is suggesting, always running scripts from the root of the workspace.

[laurentsimon]

Yeah, I thought about that. We could probably set the working dir.

I think it would be somewhat clearer, but feel free to push back: we know this one works

I'd like to push back for the reasons in other comments :)

[ianlewis]

Fyi, Actions don't support working-directory, so setting it will add inconsistency across scripts and Actions well

it's moot to the argument at this point. but actions do support working-directory

[laurentsimon]

Fyi, Actions don't support working-directory, so setting it will add inconsistency across scripts and Actions well

it's moot to the argument at this point. but actions do support working-directory

Last time I tried it was an invalid workflow, IIRC. I think it only works for the run command. I think that's what the document hints at with where the command is run.