Comments (12)
I want to use sync/atomic
package to make it thread-safe.
This explain why I used int32
, it's because we don't have a atomic.AddInt
func, just atomic.AddInt32
.
Also it requires a pointer.
EDIT: but of course, we can change that to use a mutex instead, or sync.Map
.
from task.
Hmm... I meant, it wouldn't work each value were set as a separated key in the context. I understood you meant this way, because them they would be immutable.
Anyway, I think copying is a best fit. 😉
from task.
Starting on a proposal/discussion of 1 (copy Task) .
ReplaceVariables
is currently called within the following functions:
Executor.runDeps(ctx context.Context, call Call) error
Executor.runCommand(ctx context.Context, call Call, i int) error
Executor.getTaskDir(call Call) (string, error)
Executor.ReplaceSliceVarialbes(call Call) ([]string, error)
Executor.getVariables(call Call) (error)
ReplaceSliceVariables
is called within:
Executor.isUpToDateStatus(ctx context.Context, call Call) (bool, error)
Executor. isUpToDateTimestamp(ctx context.Context, call Call) (bool, error)
While it probably does not resolve the full problem, if we made a copy of task, all of the methods above would have at least one true point below:
- not need to call ReplaceVariables
- not need access to Executor
- not need access to Call
- not be needed at all
- can be moved to Task
This is how a copy of Task might look like in code (function headers only):
type Call struct {
Cmd string `yaml:"sh"`
Task string `yaml:"task"`
Vars Vars `yaml:"vars"`
Env Vars `yaml:"env"`. // Maybe add this?
}
type Task struct {
Deps []Call // _could_ allow Cmd's as deps
Cmds []Call
...
e *Executor // Maybe add this? Should be null if the task has not been compiled.
}
// CompiledTask returns a copy of the task with the given name with all templates
// compiled for all fields, and Dir resolved to an (absolute) directory.
func (e *Executor) CompiledTask(name string, vars Vars) (*Task, error) {...}
// IsUpToDate returns true if the task is up-to-date, or false if it's not. If an
// error is returned, the first parameter will always be false.
func (t Task) IsUpToDate() (bool, error) {...}
// task private helper functions for the above public method.
func (t Task) isUpToDateTimestamp() (bool, error) {...}
func (t Task) isUpToDateStatus() (bool, error) {...}
// Run a compiled task instance. Returns an error if the task is not compiled.
func (t Task) Run() error {...}
// IsTask returns true if c represents a task call.
func (c Call) IsTask() bool {...}
// CompiledTask is a shortcut for e.CompiledTask(c.Task, c.Vars).
func (c Call) CompiledTask(e *Executor) (*Task, error) {...}
// Cmd returns a prepared os/exec.Cmd that can be used for execution.
func (c Call) Cmd(env Vars) (exec.Cmd, error) {...}
from task.
Since I suggest using Call
for both Cmd and Deps, and potentially allow shell commands as deps, we might also want the following convenience function:
func (c Call) IsUpToDate(e *Executor) (bool, error) {
if !c.IsTask {
return false, nil
}
t, err := c.CompiledTask(e)
if err != nil {
return false, err
}
return t.IsUpToDate()
}
from task.
I agree with these proposals.
We should do this in small steps, and not once, to prevent breakage. 💣💥
Maybe we should fix #40 before start this. I already could reproduce it, but don't have a fix yet.
from task.
I think fixing #40 and add a new test for it first would be nice, if we can figur out the cause.
As for the refactor, if some of it can be done in small steps, that would be cool, but I would not be to concerned about doing the concrete proposal for 1 in one big bang as long as it can be properly tested and rewiewed - but it probably do need more tests, perheps also some unittests of particular functions.
from task.
Comment to your commit, if you make the call-count, not to use pointers but just be a map[string]int
, you don't need to worry about preallocating the integers, as things will just work:
I also don't see a that there is reason for making this interger an int32
rather than just an int
. int
is usually a bit faster as it will match the target architecture.
from task.
I should have read the code better before I commented. Otherwise the changes looks much better.
from task.
Happy you invite me for reviews if you want btw., and do some manual testing for this refactoring in particular.
from task.
@smyrman Sure, testing is always welcome. 😄
I really liked the idea of cloning the task. Some other small refactorings will be easier now.
from task.
We already use context.Context for our calls. context.Context wold also be a perfect candidate for passing variables to (sub-task), as it is always forked when modified, similar to how a shell is forked when you run a command or sub-shell.
I think that wouldn't work, because we'd need to convert context.Context
is an map[string]string
to pass to templates, and that's not easily achievable because we can't know what keys are available inside a context.
from task.
I think that wouldn't work, because we'd need to convert context.Context is an map[string]string to pass to templates, and that's not easily achievable because we can't know what keys are available inside a context.
I meant 1 (copy of task) and 2 (use context) as two alternatives, so happy we go with 1. I think that's the best fit. However, I assure you it wold work :-)
If you are interested in using Context for storing variables (for some other project perhaps), the way I understand the idiotic pattern for best usage is something like this:
type ctxKey int // private type to avoid key collision with other packages using context...
// private keys to prevent miss-usage outside of package.
const (
ctxKeyVars ctxKey = iota
ctxKeyEnv
func ContextWithVars(ctx context.Context, vars Vars) context.Context
return context.WithValue(ctxKeyVars, vars)
}
func VarsFromContext(ctx) Vars {
i := ctx.Value(ctxKeyVars).
if i == nil {
// when no value was set for ctxKeyVars.
return nil
}
// only ContextWithVars may possible set this value,
// so no reason to check for a second ok parameter
// during type-cast.
return i.(Vars)
}
That said, it's probably still better to pass explicit parameters whenever possible, or at least according to some of the popular golang bloggers.
from task.
Related Issues (20)
- Request: Docs section listing all template functions available HOT 4
- Vars can be included in other vars, but env vars not in other env vars
- Add TASK_EXECUTABLE_PATH variable
- Suppress signal received message HOT 1
- Add support for faint colors HOT 2
- Multiline commands for dynamic vars HOT 2
- Declaration order matters when nesting dynamic vars HOT 1
- [Bug] Global Taskfile with local dotenv file not loading Taskfile variables using `USER_WORKING_DIR` path reference
- Define ENV vars not static HOT 3
- `--watch` should start new process after old process stopped HOT 1
- unable to use ( or ) when calling powershell
- align target hints by column HOT 1
- Migrate from `sprig` (`slim-sprig`) to `sprout`?
- Regression - env var yaml integer no longer exported to enviroment HOT 1
- Included variables using common names are getting overriden HOT 3
- `defalt` and `join` not available in ref variables
- When includes are used, the value of the variable is not constant and changes randomly. HOT 5
- Long number string variables are now converted to scientific notation, breaking existing tasks that expect the number as is HOT 4
- feature request: Unit Testing Taskfiles HOT 1
- When multiple includes are used sometimes variables randomly get lost HOT 4
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from task.