nuxt / rfcs

RFCs for changes to Nuxt.js
98 stars 2 forks source link

Prevent nuxt from running more than once #23

Closed pimlie closed 5 years ago

pimlie commented 5 years ago

Why?

Currently you can run nuxt build and nuxt generate multiple times when running from different console windows, even though nuxt doesnt support that as it removes the whole buildDir when a build is started or the generate.dir when you run the generate command.

This becomes a potential problem when you start automating (non-ci?) deployments. Currently the easiest way to automate nuxt deployment is to add some cronjobs, but cronjobs have by default no prevention for jobs running more than once. So if you set nuxt to run every 5 minutes but building takes more than 5 minutes you will have problems (the 5min is just for the example ofc and shouldnt be used normally)

Why not as a module/plugin?

As running nuxt build (with the same buildDir) twice at the same time is not supported and will never work either, we should always try to prevent this from happening.

Proposal

Use a lock file which we can check to prevent nuxt build/generate from running twice

Last year I already made a pull request for this (see: https://github.com/nuxt/nuxt.js/pull/4239), but I closed that one because at the time there were quite some other changes to the cli commands and from my discussion with pi0 I realised this feature required more thinking and decision making. Also I included other changes to the cli command in that pr as well which probably made the scope of the pr a bit too broad.

Main difficulties / key decisions to make

pi0 commented 5 years ago

Is adding a lock file really the best way to prevent nuxt from running twice?

I think so. This is the most standard way of preventing that.

One lock file or lock file per build/generate? Use a options.lock.build/generate key or a options.build/generate.lock key? Should we support other lock paths then the ${buildDir}.lock and ${generate.dir}.lock? Would that ever be useful?

For naming, I suggest using .nuxt-{hash}.lock where hash consists of:

This way we can guarantee a safe approach to prevent conflicts.

Adding mode (build/dev/gen/start) adds any value but opens possibilities to happen this problem again in some scenarios.

Do we lock inside or outside buildDir and generate.dir? My preference would be to lock outside as locking should be independent of any stuff that eg nuxt build does with the .nuxt folder (by default proper-lockfile when called with buildDir would create a lock folder named .nuxt.lock) If there are plans to make nuxt build to be more atomic (which would be very welcome) by building to a temporary folder and then rename/move that temporary folder to buildDir, is this locking then still required? Or should we then have an additional option to only warn when nuxt runs twice? Others?

It makes sense to create lock a file in an isolated and git-ignored directory which is for-now buildDir (.nuxt). It is true that stuff inside .nuxt would be better to separate - with temporary names. But as of older PR, I still think this is not a good reason to create lock file outside of buildDir. This problem has different scope and would be better to address in another RFC.

For now, I would suggest creating the lock file with the naming strategy above inside either:

manniL commented 5 years ago

We should think about what happens when aborting the build/generate too. In that case, the file should be easy to delete (ideally via dialogue)

pi0 commented 5 years ago

@manniL For this, the lock file contains PID of the process. We can validate lock file by checking PID existence. However, I agree. A simple message about the solution would also work.

pimlie commented 5 years ago

@manniL If you say abort then that sounds like you mean a soft-exit like ctrl+c (SIGINT/SIGTERM)? Actually proper-lockfile already takes care of that: https://github.com/moxystudio/node-proper-lockfile#graceful-exit (they use https://github.com/tapjs/signal-exit to listen for process exit events)

But indeed, we need some invalidation method for when the process was killed by SIGKILL or due to VM fatal error.

@pi0 Makes sense, in that case I think I like node_modules/.cache/nuxt/ the most as it at least keeps everything in the project dir. Maybe we should add a config option for a 'temporary nuxt dir' which can by default point to the previous mentioned but a user can override it and point it somewhere else? That way we have a single folder which we could use for more stuff and its not something dedicated to locks. And when the .nuxt folder has been repurposed we could change it to .nuxt/tmp/ or .nuxt/locks or something.

Another thing we discussed in my original pr was whether this should be part of nuxt-cli or whether it should be added as a utility method. I think I agree with you it would be nicer as a re-usable utility method, but do you still agree with that too? Also I am not sure which package would be best for this. A potential problem with also provide locking functionality when Nuxt is used programmitically is the invalidation method for when an (outdated) lock file already exists. I am not in favor of forcing dialogs when someone uses Nuxt programmatically so I think actual using those locking utilities should be done in nuxt-cli (and not in eg the Builder class). If someone who uses Nuxt programmatically also wants to support locking, I think there is no other way then that they need to implement those utility classes themselves as well so we dont enforce some dialogs upon them.

pimlie commented 5 years ago

As the pr has been merged, this rfc can now be closed. Thanks