Tonkpils / snag

Automatic build tool for all your projects
MIT License
32 stars 4 forks source link

Snails #64

Open robertoandrade opened 7 years ago

robertoandrade commented 7 years ago

ménage à trois of snails

Working with @CrystalCodes01 on SailsJS project I came to introduce her to snag so she didn't have to keep on running sails lift every time she modified a file in the project.

Problem is that every time she modifies a file in the project snag doesn't seem to pick it up and rerun sails lift, perhaps because the first time you run snag the initial sails lift never completes, given it's basically running node app.js behind scenes and it keeps on running the node app until you interrupt it.

This is slowing down her development quite significantly as she has to constantly CTRL+C to interrupt either sails lift or snag running and rerun the command again upon every modification to the project, hence the name of the bug and project: SNAILS (SNAG+SAILS) 😆 (BTW her project also has SLUGs, so the name is gonna "stick" - puns totally intended)

I've put together a quick sample using sails new and created a .snag.yml file to ignore .git and .tmp (generated by sails) as well as any node_modules. You'll notice when checking out the project (make sure you have SailsJS installed: sudo npm install -g sails) that upon running snag it runs the bash script run.sh which is basically performing sails lift and pumping the output of it into a log file under .tmp/log. If you keep tailing the log to see what sails is doing, you'll notice the cool looking sail and notice of the app running:

info: Starting app...

info: 
info:                .-..-.
info: 
info:    Sails              <|    .-..-.
info:    v0.12.13            |\
info:                       /|.\
info:                      / || \
info:                    ,'  |'  \
info:                 .-'.-==|/_--'
info:                 `--'-------' 
info:    __---___--___---___--___---___--___
info:  ____---___--___---___--___---___--___-__
info: 
info: Server lifted in `/home/chronos/Documents/snails`
info: To see your app, visit http://localhost:1337
info: To shut down Sails, press <CTRL> + C at any time.

As soon as you touch any file in the project or add new ones, one would expect snag to kill that running process and rerun the build as declared in .snag.yml but it looks like it's "stuck" running the initial build so it never does what I expected it to.

If this is a design issue, I was wondering if we could have a flag or option that can be set in the manifest file that determines that builds should be interrupted if while they are running files are modified, which should trigger new builds.

Let me know what you guys think @zabawaba99 @Tonkpils

Tonkpils commented 7 years ago

@robertoandrade thanks for that details and very pun-filled issue 🐌. Seems like it should work™, I'll have a look at it. What OS are you running?

robertoandrade commented 7 years ago

Tried both on OSX and Ubuntu. On Fri, Mar 31, 2017 at 9:07 AM Leo Correa notifications@github.com wrote:

Seems like it should work, I'll have a look at it. What OS are you running?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Tonkpils/snag/issues/64#issuecomment-290705715, or mute the thread https://github.com/notifications/unsubscribe-auth/AATd-SyTO5C851kLvEB9ljwa1kOgFjZqks5rrPk5gaJpZM4Mvbql .

Tonkpils commented 7 years ago

So the issue is that we expect the commands to finish doing their job before allowing them to be killed as seen by this mutex https://github.com/Tonkpils/snag/blob/master/vow/promise.go#L119 . Removing the mutex allows rebuilding without waiting for the command to finish but it seems sails is modifying the updated at time or somehow touching a view file in the layouts folder when starting the app causing an endless loop which is odd for sails.

digging into it though, I found out that sails seems to come with auto reload on the app so you don't need to restart the server when code changes as it will automatically reload it in development, a la rails. @zabawaba99 any idea why we need to wait for the process to finish?

robertoandrade commented 7 years ago

I thought all it did was put stuff into the .tmp folder, so you're saying it touches files under other folders when lifting?

robertoandrade commented 7 years ago

Removing the mutex allows rebuilding without waiting for the command to finish

Does this mean the old process gets killed and you wait for it to be completely killed before you start the new build?

Should this be a configurable thing we can set in the .snag.yml for each project instead of always wait or not?

robertoandrade commented 7 years ago

If you allow https://github.com/Tonkpils/snag/blob/master/builder.go#L196 to be called again when it detects a file system change, it looks like it should stop the current commands that are pending, releasing the lock, no?

zabawaba99 commented 7 years ago

any idea why we need to wait for the process to finish?

We were doing that so that processes have a change to cleanup what they were doing and also so that they don't stomp on each other. I believe we were seeing an issue initially where there were two test processes that were running and outputting logs all together.


It looks like we can get away with using a sync.RWMutex and use p.cmdMtx.RLock() instead of .Lock() since we aren't necessarily mucking with the variable but only reading it.