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

Snails #64

Open
robertoandrade opened this issue Mar 31, 2017 · 7 comments
Open

Snails #64

robertoandrade opened this issue Mar 31, 2017 · 7 comments

Comments

@robertoandrade
Copy link


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
Copy link
Owner

Tonkpils commented Mar 31, 2017

@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
Copy link
Author

robertoandrade commented Mar 31, 2017 via email

@Tonkpils
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

robertoandrade commented Mar 31, 2017

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
Copy link
Author

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
Copy link
Collaborator

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.

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

No branches or pull requests

3 participants