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

Multi-command scripts #7

Closed
probins opened this issue Nov 27, 2020 · 24 comments
Closed

Multi-command scripts #7

probins opened this issue Nov 27, 2020 · 24 comments

Comments

@probins
Copy link
Contributor

probins commented Nov 27, 2020

I've been trying out the script runner. I have what's probably a common setup, where I convert md files to html, and then want to run other scripts to create other parts of the site. So the script looks something like:

mkdir newdir
cd newdir/
deno run xxx

I put this in a separate .sh file, which works fine. But I would prefer to include this in the Lume config file, and I can't find a way to do this. If I put the cmds in one line separated by ;, then it makes dirs called newdir;, cd, etc. The same with separating the cmds with &&. I also tried using the sequential option, but it looks like it tries to cd before mkdir has completed, so that doesn't work either.

Any ideas?

@oscarotero
Copy link
Member

Mmm, looks like multi commands are not supported by Deno.run

Deno.run({ cmd: ["mkdir", "newdir", "&&", "cd", "newdir", "&&", "ls -al"]})

And executing them in different subprocesses does not follow the new cwd after the cd command:

Deno.run({ cmd: ["mkdir", "newdir"]})
Deno.run({ cmd: ["cd", "newdir"]})
Deno.run({ cmd: ["ls -al"]})

I have to investigate how to fix this.

oscarotero added a commit that referenced this issue Nov 28, 2020
@oscarotero
Copy link
Member

Ok, I've released 0.9.6 that fixes this.
You can upgrade easily with lume --upgrade

@probins
Copy link
Contributor Author

probins commented Nov 28, 2020

I just tried lume --upgrade but it fails with

error: Import 'https://dev.jspm.io/npm:es-abstract@1/2020/RequireObjectCoercible?dew' failed: 404 Not Found
    at https://dev.jspm.io/npm:object.values@1?dew:8:0

Not sure if this is related, but as of https://jspm.org/jspm-dev-release dev.jspm.io is replaced by jspm.dev.

@oscarotero
Copy link
Member

Oh, I didn't know about jspm.dev. Thanks, I'll update these dependencies.

Anyway, lume --upgrade should work even if some of these packages fail, because internally what it does is a deno cache of all packages. Execute lume -v and if it returns v0.9.6 you have lume upgraded.

@probins
Copy link
Contributor Author

probins commented Nov 28, 2020

I tried this, it returned v0.9.6, and I ran it, but there was still the same issue. After trying various things, I deleted the cache and tried again. This time it correctly split the commands, so this seems to be the issue I had before with the cache not being cleared correctly. The mkdir works correctly, but cd brings No such file or directory (os error 2). This is not one of the newly created dirs, but one that already existed. I added a pwd to see if it was somehow in the wrong dir, and a ls, but it's in the right dir, and the dir exists. So I'm not sure what the problem is. Something is going on here which I don't understand.

I'll go back to using a .sh file for the moment, as that does work.

@probins
Copy link
Contributor Author

probins commented Nov 28, 2020

could you perhaps try mkdir newdir && cd newdir on your machine and see if it works?

@oscarotero oscarotero reopened this Nov 28, 2020
@oscarotero
Copy link
Member

I can run this script without problems:

site.script("multi",
  "mkdir newdir",
  "cd newdir",
  "touch newfile",
  "ls -al",
  "cd ..",
  "rm -rf newdir"
);
lume --run multi
⚡️ mkdir newdir
⚡️ cd newdir
⚡️ touch newfile
⚡️ ls -al
total 0
drwxr-xr-x  3 oscarotero  staff   96 28 nov 19:32 .
drwxr-xr-x  7 oscarotero  staff  224 28 nov 19:32 ..
-rw-r--r--  1 oscarotero  staff    0 28 nov 19:32 newfile
⚡️ cd ..
⚡️ rm -rf newdir

And creating an unique commands with && works fine too:

site.script("multi", "mkdir newdir && cd newdir && touch newfile && ls -al && cd .. && rm -rf newdir");

Could I see a command that is not working to you.

@probins
Copy link
Contributor Author

probins commented Nov 28, 2020

I copied your test exactly, and the full error stack I get is:

lume --run multi
⚡️ mkdir newdir
⚡️ cd newdir
error: Uncaught (in promise) NotFound: No such file or directory (os error 2)
    at processResponse (core.js:223:11)
    at Object.jsonOpSync (core.js:246:12)
    at opRun (deno:cli/rt/40_process.js:19:17)
    at Object.run (deno:cli/rt/40_process.js:103:17)
    at Scripts.#runCommand (https://deno.land/x/lume@v0.9.6/scripts.js:52:26)
    at Scripts.#runScript (https://deno.land/x/lume@v0.9.6/scripts.js:44:28)
    at Scripts.run (https://deno.land/x/lume@v0.9.6/scripts.js:21:44)
    at async Scripts.run (https://deno.land/x/lume@v0.9.6/scripts.js:21:23)
    at async cli (https://deno.land/x/lume@v0.9.6/cli.js:162:21)

I tried with sleep 2 after the mkdir, and also tried ls -l, which lists newdir as expected. newdir has the correct owner etc, so I don't think it's a permission thing. I am mystified. ☹️

@probins
Copy link
Contributor Author

probins commented Nov 28, 2020

I wondered if it might be some peculiarity in my machine's setup, so I pushed to GitLab's CI, using alpine-deno docker container and

deno run -q --unstable -A https://deno.land/x/lume/cli.js

so using the latest version, and I get the same problem. Hmm.

@oscarotero
Copy link
Member

mm, no idea.
Could you help me to debug lume in your local machine?
You can clone this repo and execute it locally:

deno run --unstable -A lume/cli.js --run multi

So you can debug the script runner. What I'm doing is detecting whether the last command is cd, so I can change the cwd of the next command. You can see here the code:
https://deno.land/x/lume@v0.9.6/scripts.js#L56

With a console.log(options) you can know if the commands are executed in the right cwd.

Maybe the problem is that you need to pass some environment variables? Deno.run has an env option so try to change the code for something like:

options.env = Deno.env.toObject();

I'm using macOS without problems, but I'm going to create a testing repository and use the github CI.
Thanks!

@probins
Copy link
Contributor Author

probins commented Nov 28, 2020

ok, I'll take a look tomorrow

@probins
Copy link
Contributor Author

probins commented Nov 29, 2020

cd won't work when split into multiple processes, as described in https://askubuntu.com/questions/481715/why-doesnt-cd-work-in-a-shell-script - the cd only applies to that process, not to the following command/process. So all the commands needed have to be in the same process (presumably macOS handles this differently).

This doesn't explain this particular issue, but I'm sure it's related somehow, and if it's not going to work in my case anyway ...

I prefer to keep the directory tidy and keep the script commands in the config file, so what I could do is change my config file to create a temp file and then execute it. Lume could do this for me, i.e. keep the array of processes for commands that are unrelated, and change the sequential option to create a (executable) file and then run it. And change the docs to explain the difference. What do you think?

@probins
Copy link
Contributor Author

probins commented Nov 29, 2020

cd isn't actually necessary, as I can do:

Deno.run({ cmd: ["ls","-al"], cwd: "mydir"})

(["ls -al"] does not work on my machine, I have to split out the params)

deno also includes all the filesystem cmds like mkdir, so I'm thinking what I want to do can be done by running a deno/js script rather than creating a subprocess. How do I do that within the config file? Can I just create a module which does that, and import it?

@oscarotero
Copy link
Member

cd won't work when split into multiple processes,

Yes, I know. This is why I detect the "cd" command to change the cwd after running the scripts, so the next processed will be runned with the new cwd. You can see it here: https://deno.land/x/lume@v0.9.6/scripts.js#L56

So, basically, what I do is:

cd mydir && ls -al is converted to two scripts:

Deno.run({ cmd: ["cd", "mydir"] });
Deno.run({ cmd: ["ls", "-al"] });

But using the same options object for two:

const options = { cwd: Deno.cwd() };

//Script 1
let cmd = ["cd", "mydir"];
Deno.run({ cmd, ...options });

//Change the cwd
if (cmd[0] === "cd") {
  options.cwd = join(options.cwd, cmd[1]);
}

//Script 2
cmd = ["ls", "-al"]
Deno.run({ cmd, ...options });

@probins
Copy link
Contributor Author

probins commented Nov 29, 2020

to answer my question about running a js script, I can create an ordinary function in the config file with for example Deno.mkdirSync("newdir");, and then for example do:

site.addEventListener("afterBuild", myFunction);

If I import my script, it's loaded on startup, which I don't want, but I can do import('./myScript.js') within myFunction. This works, but my script is currently set up to use Args, which a simple import won't have. So I'm thinking I will change myScript to export a function, import that into the config file, and then run it in myFunction with the appropriate param. I think that should work. That would remove the need for any subprocesses at all.

https://deno.land/manual/examples/subprocess has an example of using Deno.run() to do deno run, but then I would be using one Deno script (Lume) to run Deno.run to run deno run - which gets really confusing. 😕

@oscarotero
Copy link
Member

Ok, I've the same issue as you in GA: https://github.com/lumeland/test/runs/1470070616#step:4:594

I'll try to fix it and, in addition, add support to assign javascript functions to scripts. So, for example, you could do this:

function myFunction () {
   // Your code here
}

site.script("my-script", myFunction);

So, you can run this function from cli lume --run my-script

@probins
Copy link
Contributor Author

probins commented Nov 29, 2020

yeah, that would make it very versatile. Especially if I could have myFunction(params) and lume --run my-script <params> would pass the params through. :-)

@probins
Copy link
Contributor Author

probins commented Nov 29, 2020

just to confirm, this works as I thought. I changed my script to export a function. This function can then be imported into the config file, and run with a param in the function called by the afterBuild listener. There's then no need for any shell scripts or subprocesses. I additionally have a small front-end for running from the command line which also imports the function and then sets the param to args[0]. If you allow --run my-script as discussed, I could then use that instead, and get rid of this front-end.

@oscarotero
Copy link
Member

Ok, new version released (v0.9.8) with the following changes:

  • Fixed cd commands in linux
  • Added support for javascript functions as scripts
  • The arguments passed after -- are stored in site.flags, so you can use it in your functions. For example:
site.script("hello", () => 
    site.flags.forEach((flag) => console.log(`hello ${flag}`))
);
lume --run hello -- world galaxy universe

@probins
Copy link
Contributor Author

probins commented Dec 1, 2020

lume --upgrade fails again at:

Check https://deno.land/x/lume/plugins/css.js
error: TypeError: Cannot read property 'kind' of undefined
    at Object.isEntityNameExpression (deno:cli/tsc/00_typescript.js:17115:21)
    at serializeMaybeAliasAssignment (deno:cli/tsc/00_typescript.js:48356:42)
    at serializeSymbolWorker (deno:cli/tsc/00_typescript.js:47874:49)
    at serializeSymbol (deno:cli/tsc/00_typescript.js:47825:38)
    at deno:cli/tsc/00_typescript.js:47800:25
    at Map.forEach (<anonymous>)
    at visitSymbolTable (deno:cli/tsc/00_typescript.js:47799:33)
    at serializeAsNamespaceDeclaration (deno:cli/tsc/00_typescript.js:48162:25)
    at serializeModule (deno:cli/tsc/00_typescript.js:48071:25)
    at serializeSymbolWorker (deno:cli/tsc/00_typescript.js:47949:25)

presumably, a TS error rather than a JS one. I'm on deno 1.5.4.

And again, deleting the cache and then reinstalling works. I need to do some research on install, so I understand what's going on here.

@probins
Copy link
Contributor Author

probins commented Dec 1, 2020

There seems to be some kind of issue with running async functions within a site function.

This is the config file I created yesterday, which works fine:

import lume from "https://deno.land/x/lume/mod.js";
import process from './tracks/process.js';
const site = lume({
  src: "src",
  dest: "public",
  prettyUrls: false
});
site.copy("density.png");
function runprocess() {
  Deno.mkdirSync("public/current");
  Deno.mkdirSync("public/2025");
  Deno.mkdirSync("public/2030");
  Deno.mkdirSync("public/2035");
  Deno.chdir("./tracks");
  process();
  process('2025');
  process('2030');
  process('2035');
}
site.addEventListener("afterBuild", runprocess);
export default site;

So it simply calls runprocess afterBuild, which creates the output dirs, switches to another dir, and runs the imported function several times for different years. The write function looks like:

  writeFile(fileName) {
    Deno.writeTextFile(fileName, JSON.stringify(this.fc))
    .then(() => {
      console.log(`${this.fc.features.length} recs saved to ${fileName}`);
    })
    .catch(err => {
      console.log(err);
    });
  }

I've now added:

function doProcess() {
  Deno.chdir("./tracks");
  process();
}
site.script('doprocess', doProcess);

so I can run process() separately without building the site. But if I do lume --run doprocess, it runs through correctly, but no files are written. I added console.log to the start of the output function, which prints ok, but no files are written and there are no save or error logs from writeTextFile. So it looks like Deno.writeTextFile is never being executed. Thinking that it might be something to do with async functions, I changed to writeTextFileSync, and this time it worked. So writeTextFileSync works, and writeTextFile doesn't. Any ideas?

@oscarotero
Copy link
Member

Ok, I'll check that.

@oscarotero oscarotero reopened this Dec 1, 2020
@oscarotero
Copy link
Member

Ok, I've found the issue.
The script runner runs the function and then execute a Deno.exit(status). Your function writeFile does not returns the promise, so it doesn't know that this is an asyncronous function, so it exits before the promise is resolved.
I've changed the logic so now it's not required to return a promise, so it should work fine.

I've also changed the way --upgrade works. Now it fetch the latest version from deno.land (https://github.com/lumeland/lume/blob/master/cli.js#L83) and install this specific version (for example: https://deno.land/x/lume@v0.9.9/cli.js) so I think this fixes the cache problems. (of course, this change will be effective after this upgrade).

@probins
Copy link
Contributor Author

probins commented Dec 2, 2020

yes, that all seems to work now, thanks. I'd suggest adding some docs/examples on how to use flags. site.flags seems to always exist, so I can just do process(site.flags[0]) and it will just use 'undefined' if there are no flags (which is what I want).

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

2 participants