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

chore(build): add go-task support #1608

Merged
merged 6 commits into from
Feb 8, 2023
Merged

chore(build): add go-task support #1608

merged 6 commits into from
Feb 8, 2023

Conversation

mritd
Copy link
Contributor

@mritd mritd commented Jan 13, 2023

add go-task support

Signed-off-by: kovacs mritd@linux.com

add go-task support

Signed-off-by: kovacs <mritd@linux.com>
@mritd mritd changed the title chore(build): add go-task support [WIP] chore(build): add go-task support Jan 13, 2023
@codecov
Copy link

codecov bot commented Jan 13, 2023

Codecov Report

Merging #1608 (97af8e3) into master (9c58278) will increase coverage by 0.28%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1608      +/-   ##
==========================================
+ Coverage   87.86%   88.15%   +0.28%     
==========================================
  Files         104      104              
  Lines        8975     8975              
==========================================
+ Hits         7886     7912      +26     
+ Misses        916      892      -24     
+ Partials      173      171       -2     
Impacted Files Coverage Δ
pkg/cluster/pool.go 94.82% <0.00%> (+9.48%) ⬆️
pkg/task/pool.go 100.00% <0.00%> (+48.38%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

build with go-task

Signed-off-by: kovacs <mritd@linux.com>
@CLAassistant
Copy link

CLAassistant commented Feb 7, 2023

CLA assistant check
All committers have signed the CLA.

support cross compile

Signed-off-by: kovacs <mritd@linux.com>
@mritd
Copy link
Contributor Author

mritd commented Feb 7, 2023

Waiting for PR #1626 to be merged, remove GCC configuration.

remove GCC build

Signed-off-by: kovacs <mritd@linux.com>
update README

Signed-off-by: kovacs <mritd@linux.com>
@mritd mritd changed the title [WIP] chore(build): add go-task support chore(build): add go-task support Feb 8, 2023
@mritd
Copy link
Contributor Author

mritd commented Feb 8, 2023

@HFO4 PR update completed.

For the github action I haven't changed (I can't test it), it may need you to modify;

By the way, go-task already provides a standard github action installation method: https://taskfile.dev/installation/#github-actions

@HFO4 HFO4 merged commit abe90e4 into cloudreve:master Feb 8, 2023
@HFO4
Copy link
Member

HFO4 commented Feb 8, 2023

Thanks!

@HFO4
Copy link
Member

HFO4 commented Feb 8, 2023

Unfortunately, after detailed tests and estimates, I need to revert this change due to the below considerations:

I will try to use goreleaser to implement the current task flow.

HFO4 added a commit that referenced this pull request Feb 8, 2023
This reverts commit abe90e4.

Revert "chore: fix env in task yaml and test new build action"

This reverts commit 7dfe8fb.

Revert "remove unused env"

This reverts commit 076aa2c.

Revert "fix: ci build failed as env in go tasks cannot be overwritten"

This reverts commit 71cc332.
@mritd
Copy link
Contributor Author

mritd commented Feb 8, 2023

I don't understand exactly what it affects.

For the environment variable, if it is obtained through $XXX in the task, it will automatically follow the os environment variable; if the {{.XXXX}} variable is used, it supports task TASK_NAME XXXX=123 way to override.

For the if judgment statement, it can be judged by the environment variable (example: https://taskfile.dev/usage/#os-specific-taskfiles), or by the status option to judge whether it needs to be executed
(https://taskfile.dev/usage/#using-programmatic-checks-to-indicate-a-task-is-up-to-date)

For more complex processing, go-task also supports embedding shell commands to complete.

According to my understanding, goreleaser is only a compilation and release tool, and does not have any task scheduling capabilities.

For github action publishing, after executing task all, all supported platform compilation files will be generated in the release directory, and github action only needs to complete the task of "publishing all files".

@HFO4
Copy link
Member

HFO4 commented Feb 9, 2023

For the environment variable, if it is obtained through $XXX in the task, it will automatically follow the os environment variable; if the {{.XXXX}} variable is used, it supports task TASK_NAME XXXX=123 way to override.

What you're describing is "variables" not "environment variables".

For the if judgment statement, it can be judged by the environment variable (example: https://taskfile.dev/usage/#os-specific-taskfiles), or by the status option to judge whether it needs to be executed
(https://taskfile.dev/usage/#using-programmatic-checks-to-indicate-a-task-is-up-to-date)

This will make the taskfile extremely complicated. The goal is simply: archive binaries to .tar.gz or .zip based on its target OS. Goreleaser can simply achieve since it has built-in archive support.

For more complex processing, go-task also supports embedding shell commands to complete.

That would make no improvement compared to original shell script approach.

According to my understanding, goreleaser is only a compilation and release tool, and does not have any task scheduling capabilities.

We don't need task scheduling capabilities in goreleaser since it has all features we needed for packaging Cloudreve.

For github action publishing, after executing task all, all supported platform compilation files will be generated in the release directory, and github action only needs to complete the task of "publishing all files".

We still need to archive it to .tar.gz or .zip. Yes, we can surely implement this with go-tasks + shell script, but that will make go-task no advantages compared to the old way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants