phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
2 stars 5 forks source link

'grunt create-sim' fails if $HOME/.phet/build-local.json doesn't exist #202

Open pixelzoom opened 3 years ago

pixelzoom commented 3 years ago

Reported by a 3rd-party in the Google Group, https://groups.google.com/d/topic/developing-interactive-simulations-in-html5/zBt-qQQVbkM?pli=1&authuser=0:

Hello,

I am setting up the development environment for the first time and I was able to start the http server and see the example sim in action, however, when I went ahead and entered the command:

grunt create-sim --repo=acceleration-lab --author="Mayank Pandey"

And I get this error:

warn: No build-local.json detected! Loading "Gruntfile.js" tasks...ERROR

Error: ENOENT: no such file or directory, open '/Users/mayankpandey/.phet/build-local.json' Warning: Task "create-sim" not found. Used --force, continuing.

I followed the instructions for Mac OS and I even tried on a windows machine, but apparently that doesn't mean anything and honestly I tried many things but I am stuck so I would really appreciate any help towards solving this issue!

I have reproduced this problem. If $HOME/.phet/build-local.json doesn't exist, then grunt create-sim (and possibly other tasks) will fail.

I don't know how to fix this problem. But I will instruct the user on how to create an empty $HOME/.phet/build-local.json, since this is a blocking issue for them.

Assigning to @ariel-phet for prioritization and assignment. Recommended high priority.

pixelzoom commented 3 years ago

My response to the user in the Google Group:

Sorry that you've encountered this problem. I've reproduced the problem, and created a GitHub issue to get it resolved. See https://github.com/phetsims/perennial/issues/202.

In the meantime, you can create an empty build-local.json by opening the Terminal application on macOS, and running this command:

echo "{}" > /Users/mayankpandey/.phet/build-local.json

Let me know if that allows you to continue working.

Chris Malley PixelZoom, Inc.

zepumph commented 3 years ago

I think that the error is coming from https://github.com/phetsims/perennial/blob/627e7f5c5e836721fbf2eb2fbbd37fb085fd64ac/js/grunt/decaf/deployDecaf.js#L28, because it is outside of a function, it executes upon import into perennial's Gruntfile.

I would recommend using buildLocal.js anyways, which has built-in grace. Over to @Denz1994 who added that logic in deployDecaf.

Denz1994 commented 3 years ago

because it is outside of a function, it executes upon import into perennial's Gruntfile.

Right, I moved these constants into the function scope of bothbuildDecaf.js and deployDecaf.js. This wasn't caught initially, because my machine has a .phet/build-local.json.

I didn't use buildLocal.js because it returns only a subset of fields from the buildLocalObject, so fields like buildLocalObject.gitRoot don't carry over. Instead, I added assertions to check for these fields where they are needed in buildDecaf.js and deployDecaf.js.

This was tested by removing my build-local.json and running grunt create-sim and grunt lint, where both tasks completed successfully. Assigning @samreid for review since we initially paired on this task.

samreid commented 3 years ago

Looks great and I also tested after renaming by local file, closing.

zepumph commented 3 years ago

I didn't use buildLocal.js because it returns only a subset of fields from the buildLocalObject, so fields like buildLocalObject.gitRoot don't carry over.

In my opinion, buildLocal.js should be the entire schema of what can be found in build-local.json on master. I think it would be better and clearer to add this field to buildLocal with a reasonable default instead of using a separate implementation of the same job. Perhaps another issue should be created for this though.

Another solution would be to alert buildLocal.js so that it returns whatever is in buildLocal, but _.extends() defaults so that you could use buildLocal.js without adding to that file, and still include the assertions where you have them.

samreid commented 3 years ago

Good point @zepumph, I agree.