Add support for 'platforms' in both task and command

[leaanthony]

This is a draft PR based on the discussions in #978. It allows limiting a task or a command to a particulate OS/Arch combination. It also validates any given OS/Arch against a list of valid values.

Example Taskfile that is valid with this PR:

version: '3'

tasks:
  build-windows:
    platforms: [windows]
    cmds:
      - echo 'building on windows'

  build-darwin:
    platforms: [darwin]
    cmds:
      - echo 'building on darwin'

  build-mixed:
    cmds:
      - cmd: echo 'building on windows/arm64'
        platforms: [windows/arm64]
      - cmd: echo 'building on linux/amd64'
        platforms: [linux/amd64]
      - cmd: echo 'building on darwin'
        platforms: [darwin]

Todo:

• Add test for testdata/platforms
• Update documentation

[pd93]

I really like this proposal and the direction this is going in. Just a couple of small comments on the draft PR :)

[leaanthony]

@pd93 - addressed comments. Feels a lot cleaner now as you suggested 👍
EDIT: I did try listing but the linter breaks with what seems like a timezone issue:

WARN [runner] Can't run linter goanalysis_metalinter: goimports: can't extract issues from gofmt diff output "--- /Users/leaanthony/Projects/task2/taskfile/platforms.go.orig\t2023-01-06 00:03:02\n+++ /Users/leaanthony/Projects/task2/taskfile/platforms.go\t2023-01-06 00:03:02\n@@ -2,9 +2,10 @@\n \n import (\n \t\"fmt\"\n-\t\"gopkg.in/yaml.v3\"\n \t\"runtime\"\n \t\"strings\"\n+\n+\t\"gopkg.in/yaml.v3\"\n )\n \n // Platform represents GOOS and GOARCH values\n": can't parse patch: parsing time "2023-01-06 00:03:02" as "2006-01-02 15:04:05 -0700": cannot parse "" as "-0700" 

[pd93]

@pd93 - addressed comments. Feels a lot cleaner now as you suggested 👍 EDIT: I did try listing but the linter breaks with what seems like a timezone issue.

@leaanthony The linting issue is just the imports not being sorted. You can fix this by running goimports -w .. This should result in the changes below:

diff --git a/taskfile/platforms.go b/taskfile/platforms.go
index c748b15..ec0f7f2 100644
--- a/taskfile/platforms.go
+++ b/taskfile/platforms.go
@@ -2,9 +2,10 @@ package taskfile
 
 import (
        "fmt"
-       "gopkg.in/yaml.v3"
        "runtime"
        "strings"
+
+       "gopkg.in/yaml.v3"
 )
 
 // Platform represents GOOS and GOARCH values
diff --git a/taskfile/task.go b/taskfile/task.go
index 12dc76b..c5a9a55 100644
--- a/taskfile/task.go
+++ b/taskfile/task.go
@@ -2,6 +2,7 @@ package taskfile
 
 import (
        "fmt"
+
        "gopkg.in/yaml.v3"
 )

Not sure what is causing you to not be able to run the linter. I haven't seen that error before.

[pd93]

@leaanthony The code layout looks good to me now. Just checked out the branch to try it out and I have a couple of observations:

1.

If we have a Taskfile like:

version: '3'

tasks:
  default:
    platforms: [linux/foo]
    #                 ^ invalid arch
    cmds:
      - echo 'building...'

The error message isn't quite right:

task: Failed to parse testdata/platforms/Taskfile.yml:
task: arch 'linux' unsupported
exit status 1

This should probably read task: Arch 'foo' is not supported. Note the capitalisation too - we've not been entirely consistent, but most of the errors in the repo are Capitalised. (Note to @andreynering, we should probably standardise this).

2.

Do we want to support something like this?

  default:
    platforms: [amd64]
    cmds:
      - echo 'building...'

Or are we happy with only supporting OS and OS/ARCH. It would be nice to add this for situations where you want to build using a different command depending on arch, but don't care about the OS. If we do add support for ARCH, we'd need to update the error handling to say its "not a valid OS or arch" when only 1 value is given.

Finally

Once we're happy with the above, we could do with adding the following too:

• Small section in the usage docs to describe the feature.
• Update the schema and API Reference (preferably with the same description in each). The items here are sorted in the same order as they appear in the Go structs.
• And finally, squash all the commits :)

Happy to help with any of this ^^^

[leaanthony]

Thanks for all the quick and informative feedback. I've pushed an update with the following changes:

• A single platform value (without a slash) can now be either OS or Arch
• Invalid single value (platforms: [foo]) produces the following error: task: Invalid OS/Arch value provided (foo)
• Trailing slash value (platforms: [linux/]) produces the following error: task: Blank Arch value provided
• Invalid Arch value (platforms: [linux/foo]) produces the following error: task: Invalid Arch value provided (foo)
• Empty value (platforms: []) runs on all platforms
• Updated test task file

I'll update the docs next 👍

[leaanthony]

Docs + Schema updated. I'm not 💯 on the schema updates so feedback appreciated 🙏

[andreynering]

Hi @leaanthony!

Thanks for your work, and quick response after code review as well!

I just merged this to master and push a commit with small improvements: 2efb353.

[leaanthony]

Brilliant! Thanks @andreynering and @pd93 for your fast and great feedback. This was a pleasure.

[dkarter]

Hey all, can you please review my PR to add platforms to the JSON schema when for is used?

#1915

I believe this was accidentally missed in this one - it still works, but my editor is providing false positive lint errors.

Thanks!