buildkite / feedback

Got feedback? Please let us know!
https://buildkite.com
25 stars 24 forks source link

The UI is lying when it uses && #502

Closed nhooyr closed 5 years ago

nhooyr commented 5 years ago

&& means the next command doesn't run if the first one fails.

With custom agents, this isn't necessarily true (In fact I believe its not true with the default command hook either). Thus && is misleading.

I'm not sure what a better alternate is. Maybe ;?

plasticine commented 5 years ago

@nhooyr Hi there—where about specifically are you referring to in the UI?

nhooyr commented 5 years ago

@plasticine my bad. I should have clarified. When condensing multiline command steps into a single line, the UI uses &&.

nhooyr commented 5 years ago

it in fact cannot also use ; either because I could have manually inserted a &&. I think its best the UI use \n to indicate a newline.

plasticine commented 5 years ago

@nhooyr OK, so you’re referring to a command in a pipeline step that looks like this;

echo "hello"
echo "world"
exit 0

...having newlines converted to posix AND_IF’s (i.e echo "hello" && echo "world" && exit 0), right?

I’m not sure I see the issue here? Could you elaborate for me, or do you just find this behaviour surprising?

nhooyr commented 5 years ago

Ah so the issue seems to be when uploading a pipeline file, the UI displays the AND_IF's but $BUILDKITE_COMMAND does not contain the AND_IF's.

plasticine commented 5 years ago

@nhooyr Right I see, $BUILDKITE_COMMAND is just the raw command, as written, without being processed. The UI is just rendering the processed version of that.

I’m going to close this one out as this is currently working as designed. 🙂

toolmantim commented 5 years ago

The default command hook does use set -e so it is effectively &&, even if it’s not the exact command that’s being used. It was confusing people without it, but I can see how if you’re doing something custom it could be confusing.

We’ll probably leave this as is, because it’s rare to do custom agent command hooks and we want to communicate the nature of multiple commands. But we’re reworking that part of the UI shortly, and discussing removing that already, so we’ll take your feedback into account.

Thanks for taking the time to let us know.

nhooyr commented 5 years ago

@plasticine @toolmantim If I enter a multiline command in the UI, it has && in $BUILDKITE_COMMAND automatically. Is that not a bug?

nhooyr commented 5 years ago

@toolmantim I've looked at the source code of the default agent, I don't see any set -e in Bootstrap's defaultCommandPhase.

toolmantim commented 5 years ago

You can find it here @nhooyr: https://github.com/buildkite/agent/blob/master/clicommand/agent_start.go#L88-L98

$ buildkite-agent start --help | grep shell
   --shell value                         The shell commamnd used to interpret build commands, e.g /bin/bash -e -c (default: "/bin/bash -e -c") [$BUILDKITE_SHELL]

The && is only a bug if you're overriding the default shell, or using your own command hook to create another interpretation of $BUILDKITE_COMMAND altogether. But it's so uncommon that && makes for a sensible default for the UI.

But like I said, it's good feedback for people doing really customised setup, and we'll take it into account when looking at the changing the design of these bits of UI. Thanks again!

nhooyr commented 5 years ago

You can find it here @nhooyr: https://github.com/buildkite/agent/blob/master/clicommand/agent_start.go#L88-L98

👍

The && is only a bug if you're overriding the default shell, or using your own command hook to create another interpretation of $BUILDKITE_COMMAND altogether. But it's so uncommon that && makes for a sensible default for the UI.

I think you're misunderstanding me.

I think it's a bug that $BUILDKITE_COMMAND contains && when I put a multiline command through the UI whereas when a pipeline is uploaded via buildkite-agent pipeline upload, $BUILDKITE_COMMAND for a multiline command has the actual newlines instead of any &&.

nhooyr commented 5 years ago

E.g. if buildkite-agent pipeline upload meant that $BUILDKITE_COMMAND replaced newlines with a newline and &&, then custom agents that used bash/sh would also work.

toolmantim commented 5 years ago

I just tried to reproduce what you're describing, but I can't seem to:

I set up a pipeline with the git URL pointing to https://gist.github.com/toolmantim/5d2eb78ef37dbd3f97a4aa1c425d3011 and with two steps: a command and a pipeline upload pipeline.yml:

image

The resulting build UI:

image

and the resulting $BUILDKITE_COMMAND environment variables (in order):

image image image
$ buildkite-agent --version
buildkite-agent version 3.8.4, build 2783

Is this different to what you're seeing? If not, could you provide me some steps to information to reproduce what you're seeing?

nhooyr commented 5 years ago

I'm sorry, was my bad. I had added the && myself without noticing earlier. Yea there is no bug then. I guess some docs for this could be nice on the custom agents page if you guys end up keeping the &&.

toolmantim commented 5 years ago

😅 No worries! I'm glad it wasn't lies, or a bug. If you could include a few more details in future bug reports it'd be much appreciated — then we can all get to the bottom of things a little quicker 🙌🏼