search

ndmitchell / rattle

Forward build system with speculation and caching
Other
102 stars 5 forks source link

cmd cannot be "cd" #5

Closed spall closed 5 years ago

spall commented 5 years ago

It seems that 'cmd "cd"' doesn't work. It produces an error like this:

rattle-test: user error (Development.Shake.cmd, system command failed Command line: fsatrace rwmdqt /tmp/extra-dir-9274181680790/fsatrace.txt -- cd Original command line: cd Exit code: 1 Stderr: )

This seems like a bug to me.

spall commented 5 years ago

I am realizing that the intention is probably to not allow this as the user should use "setCurrentDirectory" so I am going to close this issue.

ndmitchell commented 5 years ago

Uses definitely shouldn't use setCurrentDirectory - that is likely to break everything! Instead users should use cmd (Cwd pathIWant) myCommand - which I pushed support for earlier today.

spall commented 5 years ago

Oh great! Thanks! Does this let the build change directories and then all subsequent commands occur in that directory?

ndmitchell commented 5 years ago

Nope, it's stateless - it applies to that cmd alone - you have to include it on every command. Having a withCwd to Rattle that added it to all inner commands would be possible. It's important not to change the directory in any multithreaded process since it's a system wide state. In Shake if you have --lint turned on then it periodically checks the current directory and throws an error if it sees it has been changed.

spall commented 5 years ago

Okay; I think adding a withCmd will be a good idea. I can probably work on that.

ndmitchell commented 5 years ago

I just pushed a patch adding withCmdOptions of the type described in another message. Using that:

withCmdOptions [Cwd "whatever"] $ do
   cmd ...
   cmd ...

And it will be equivalent to adding Cwd "whatever" onto all the commands.