`new-package` command fails on "Adding preview folder"

[pete-murphy]

~/Code/elm
❯  elm-review new-package
✔ Your GitHub username: … pete-murphy
✔ The package name (starting with "elm-review-"): … elm-review-elm-css-migration
✔ The license for your package: … MIT
✔ Name of the rule (ex: No.Doing.Foo): … ElmCssMigration
✔ Choose the type of rule you want to start with: › Project rule
✔ Adding elm.json
✔ Adding package.json
✔ Adding elm-tooling.json
✔ Adding README
✖ Adding preview folder
-- UNEXPECTED ERROR ------------------------------------------------------------

I ran into an unexpected error. Please open an issue at the following link:
  https://github.com/jfmengels/node-elm-review/issues/new

Please include this error message and as much detail as you can provide. If you
can, please provide a setup that makes it easy to reproduce the error. That will
make it much easier to fix the issue.

Below is the error that was encountered.
--------------------------------------------------------------------------------
Error: EACCES: permission denied, open '/Users/pete/Code/elm/elm-review-elm-css-migration/preview/src/ReviewConfig.elm'
    at Object.writeFileSync (node:fs:2367:20)
    at createProject (/nix/store/mb9n2clplvhm81mlrkyjc1yn7jxsfiyb-elm-review-2.12.0/lib/node_modules/elm-review/lib/new-package.js:203:6)
    at async Object.create (/nix/store/mb9n2clplvhm81mlrkyjc1yn7jxsfiyb-elm-review-2.12.0/lib/node_modules/elm-review/lib/new-package.js:78:3)

Version info:

❯ elm-review --version
2.12.0
❯ readlink $(which elm-review)
/nix/store/r0l047mhzz7lz3dsvh2851776j8fvl49-home-manager-path/bin/elm-review

This is likely specific to Nix-installed elm-review, because if I install the same version via NPM I'm able to run the elm-review new-package command successfully.

What I think is happening: I've noticed when running elm-review init (using the Nix-installed elm-review), that the generated ReviewConfig.elm is read-only. I'm guessing this has to do with these lines

node-elm-review/lib/init.js

Lines 197 to 202 in 18e38de

	function createReviewConfig(directory, template) {
	fs.copyFileSync(
	path.join(__dirname, '../init-templates/', template),
	path.join(directory, 'ReviewConfig.elm')
	);
	}

copying from the install directory. I think the install directory will be read-only when elm-review is installed via Nix, because all files in Nix store are read-only from what I understand, and the copied files will also copy the permissions.

See the permissions for the ReviewConfig created by Nix-installed elm-review

.r--r--r-- 443 pete  8 Dec 12:26 ReviewConfig.elm

compared to NPM-installed

.rw-r--r-- 435 pete  8 Dec 12:33 ReviewConfig.elm

The new-package script then tries to write to that file, in these lines

node-elm-review/lib/new-package.js

Lines 202 to 206 in 5f2fcc3

	const previewReviewConfig = fs.readFileSync(previewReviewConfigPath, 'utf8');
	fs.writeFileSync(
	previewReviewConfigPath,
	previewReviewConfig.replace(/RULENAME_TO_REPLACE/g, ruleName)
	);

as reported by the error.

[pete-murphy]

Possible fix could be changing createReviewConfig to set write permissions for owner (and read-only otherwise)

function createReviewConfig(directory, template) {
  const destinationPath = path.join(directory, 'ReviewConfig.elm');
  fs.copyFileSync(
    path.join(__dirname, '../init-templates/', template),
    destinationPath 
  );
  fs.chmodSync(destinationPath, 0o644); // Set permissions to be writable by owner
}

[jfmengels]

Hi @pete-murphy 👋

Thanks for the detailed issue! I have very little knowledge of Nix, so pardon my confusion. I'm still confused as to which files (or folders?) require write permissions. The write permission not be set on the created preview/ folder, is that correct?

Do you know if other files created by elm-review new-package (or new-rule) would suffer from the same issue? Are you able to comment out the code for the creation of the preview folder and see if the problem appears for other files?

[pete-murphy]

which files (or folders?) require write permissions. The write permission not be set on the created preview/ folder, is that correct?

The preview/ folder is created by a call to mkdir which seems to create directories with owner-writable permissions. I think the problem is just that anything copied from the Nix store will be read-only (any calls to copyFile(Sync), so my suggested fix above would not be enough).

Are you able to comment out the code for the creation of the preview folder and see if the problem appears for other files?

Just removing these lines

  fs.writeFileSync(
    previewReviewConfigPath,
    previewReviewConfig.replace(/RULENAME_TO_REPLACE/g, ruleName)
  );

from the elm-review/lib/new-package.js in the Nix store allows new-package to succeed

❯ elm-review new-package
✔ Your GitHub username: … p
✔ The package name (starting with "elm-review-"): … elm-review-x
✔ The license for your package: … BSD-3-Clause
✔ Name of the rule (ex: No.Doing.Foo): … X
✔ Choose the type of rule you want to start with: › Project rule
✔ Adding elm.json
✔ Adding package.json
✔ Adding elm-tooling.json
✔ Adding README
✔ Adding preview folder
✔ Adding rule - X
✔ Adding .gitignore
✔ Adding GitHub Actions
✔ Adding LICENSE npx license BSD-3-Clause
✔ Adding elm-review configuration
✔ Adding maintenance scripts

All done! ✔

I created a maintenance/MAINTENANCE.md file which you should read in order to learn what the next steps are, and generally how to manage the project.

I hope you'll enjoy working with elm-review! ❤

But looking at the permissions, any files that were copied are indeed missing writable permissions:

❯ tree -p elm-review-x
[drwxr-xr-x]  elm-review-x
├── [-rw-r--r--]  LICENSE
├── [-rw-r--r--]  README.md
├── [drwxr-xr-x]  elm-review-package-tests
│   ├── [-r--r--r--]  check-examples-were-updated.js
│   ├── [-r--r--r--]  check-previews-compile.js
│   └── [dr-xr-xr-x]  helpers
│       ├── [-r--r--r--]  ansi.js
│       └── [-r--r--r--]  find-configurations.js
├── [-rw-r--r--]  elm-tooling.json
├── [-rw-r--r--]  elm.json
├── [drwxr-xr-x]  maintenance
│   ├── [-r--r--r--]  MAINTENANCE.md
│   └── [-r-xr-xr-x]  update-examples-from-preview.js
├── [-rw-r--r--]  package.json
├── [drwxr-xr-x]  preview
│   ├── [-rw-r--r--]  elm.json
│   └── [drwxr-xr-x]  src
│       └── [-r--r--r--]  ReviewConfig.elm
├── [drwxr-xr-x]  review
│   ├── [-rw-r--r--]  elm.json
│   └── [drwxr-xr-x]  src
│       └── [-rw-r--r--]  ReviewConfig.elm
├── [drwxr-xr-x]  src
│   └── [-rw-r--r--]  X.elm
└── [drwxr-xr-x]  tests
    └── [-rw-r--r--]  XTest.elm

10 directories, 17 files

I think what I would expect to see is:

  ❯ tree -p elm-review-x
  [drwxr-xr-x]  elm-review-x
  ├── [-rw-r--r--]  LICENSE
  ├── [-rw-r--r--]  README.md
  ├── [drwxr-xr-x]  elm-review-package-tests
- │   ├── [-r--r--r--]  check-examples-were-updated.js
- │   ├── [-r--r--r--]  check-previews-compile.js
- │   └── [dr-xr-xr-x]  helpers
- │       ├── [-r--r--r--]  ansi.js
- │       └── [-r--r--r--]  find-configurations.js
+ │   ├── [-rw-r--r--]  check-examples-were-updated.js
+ │   ├── [-rw-r--r--]  check-previews-compile.js
+ │   └── [drwxr-xr-x]  helpers
+ │       ├── [-rw-r--r--]  ansi.js
+ │       └── [-rw-r--r--]  find-configurations.js
  ├── [-rw-r--r--]  elm-tooling.json
  ├── [-rw-r--r--]  elm.json
  ├── [drwxr-xr-x]  maintenance
- │   ├── [-r--r--r--]  MAINTENANCE.md
- │   └── [-r-xr-xr-x]  update-examples-from-preview.js
+ │   ├── [-rw-r--r--]  MAINTENANCE.md
+ │   └── [-rwxr-xr-x]  update-examples-from-preview.js
  ├── [-rw-r--r--]  package.json
  ├── [drwxr-xr-x]  preview
  │   ├── [-rw-r--r--]  elm.json
  │   └── [drwxr-xr-x]  src
- │       └── [-r--r--r--]  ReviewConfig.elm
+ │       └── [-rw-r--r--]  ReviewConfig.elm
  ├── [drwxr-xr-x]  review
  │   ├── [-rw-r--r--]  elm.json
  │   └── [drwxr-xr-x]  src
  │       └── [-rw-r--r--]  ReviewConfig.elm
  ├── [drwxr-xr-x]  src
  │   └── [-rw-r--r--]  X.elm
  └── [drwxr-xr-x]  tests
      └── [-rw-r--r--]  XTest.elm
  
  10 directories, 17 files

[pete-murphy]

the problem is just that anything copied from the Nix store will be read-only (any calls to copyFile(Sync), so my suggested fix above would not be enough).

Maybe another possibility is to avoid using copyFile directly, and just do read then write. So changing copyFiles to

async function copyFiles(srcFolder, destFolder, files) {
  const promises = files.map(async (file) => {
    const src = path.join(srcFolder, file);
    const srcContents = await readFile(src);
    const dest = path.join(destFolder, file);
    await fsp.writeFile(dest, srcContents);
  });

  await Promise.all(promises);
}

And use that in place of these

lib/new-package.js
299:  await FS.copyFiles(
341:  await FS.copyFiles(
353:  await FS.copyFiles(

lib/init.js
199:  fs.copyFileSync(

This way we're just copying file contents and not the permissions.