Add a wrapper script to keep ./solcjs working

[cameel]

After #587 anything that relies on executing specifically ./solcjs is broken. For example in the main repo external tests (e.g. the Colony test) are failing due to this.

The right solution is to run solcjs via npm exec but I think this is might still break some workflows and we should wait with it until a breaking release. This is expecially because apparently the file in node_modules/.bin/ does not get created when you clone the repo and run npm install so npm exec is unusable in that scenario. I suspect the file in node_modules/.bin/ might only be created for packages installed as dependencies.

[axic]

We discussed this and I think this mostly a fault of the Solidity tests. Now if for some reason the file is not included in bin/solcjs that is a bug.

[cameel]

Yeah, it's not. And changing it to solcjs on files list in package.json does not help either.

[axic]

Clarification after some discussion: this is not a problem in solc-js, but rather that non-standard way Solidity tests use solc-js.

Solidity tests do a clone, then npm install and use the file directly and not via the installation. npm install also doesn't create the .bin symlink.

[cameel]

The thing is, I'm not sure all users realize that we consider this usage non-standard. I certainly did not :)

[axic]

The thing is, I'm not sure all users realize that we consider this usage non-standard. I certainly did not :)

Well, you are not supposed to clone an npm package and use internals. You are supposed to install it as a dependency or globally.

[ekpyron]

Adding a symlink does not hurt in any case.

[axic]

Adding a symlink does not hurt in any case.

After #566 it will be more complicated. The symlink will need to point to dist/solc.js, but also likely will need to run npm build after npm install.

[cameel]

I just checked and it does require npm run build. But there some other commands (e.g. npm run test) that will run build automatically so I think a symlink might still be useful to keep some workflows working. I don't see that much downside to having it and we can remove it in 0.9.

[stephen-slm]

npm link will do link between the bin and the current directly. e.g making solcjs work as expected. I think this is is the perferred way of handling local packages which are node related. It will do what you are looking for and allow solcjs to be ran anywhere.

The documentation could be updated if they are working with it locally to be something like:

npm run build

...

npm link

...

solcjs ...

[cameel]

But npm link tries to put the link into global dirs. If I'm not installing the package globally (e.g. I have multiple copies and it's just one of them) that's not what I want. I want a link somewhere in node_modules/.bin/ so that I can run it with npx.

The documentation could be updated if they are working with it locally to be something like:

We actually don't have any documentation on development usage. It all assumes you're dealing with a package installed via npm currently. Most projects have some info on building and running tests and I think we should have that too. That's a separate issue though.

[stephen-slm]

But npm link tries to put the link into global dirs. If I'm not installing the package globally (e.g. I have multiple copies and it's just one of them) that's not what I want. I want a link somewhere in node_modules/.bin/ so that I can run it with npx.

The documentation could be updated if they are working with it locally to be something like:

We actually don't have any documentation on development usage. It all assumes you're dealing with a package installed via npm currently. Most projects have some info on building and running tests and I think we should have that too. That's a separate issue though.

Makes sense,

This could be a npm script or as well like npm start which will execute the typescript directly?

[cameel]

If those can take parameters, accept standard input, itp. then I guess it would work.

Still, the point of the PR is to keep it working the old way until a breaking version.

[cameel]

I have now replaced the symlink with a small wrapper script that will run build if necessary and then execute solc.js from dist/.

[coveralls]

Coverage remained the same at 83.59% when pulling fa1bbab on solcjs-compatibility-symlink into 08ae214 on master.

[cameel]

@axic What do you think about the current version with a script rathter than a symlink. Do you have anything against merging it?

We were just discussing backwards-compatibility on Gitter and both @ekpyron and @stephensli agree that it would be nice to have at least this bit just for people who simply do download source && ./solcjs.

[cameel]

Updated the wrapper script to make it portable. Uses sh rather than bash now.

[chriseth]

Is there a way to make this not depend on the cwd?

[stephen-slm]

This was raised here #599 about flattening the directory and build entirely, but generally building into a dist folder is preferred and easier to manage.

[cameel]

Is there a way to make this not depend on the cwd?

Yeah. At least as long as you want it just in the simple case of running the script via ./solcjs and not sourcing it or invoking in some other way - handling that in a portable way is a pain.

[chriseth]

I don't understand - is there an easy and portable way to at least solve it for some cases it can be invoked in?

[cameel]

Yes, that's what I meant. For invocation via ./solcjs it's simple, I'll add it. It might just not work reliably in corner cases though.

[cameel]

Updated. Now it should be independent of where you call it from.

[chriseth]

shouldn't this be

Suggested change

	[ -e "$script_dir/dist/solc.js" ] || npm run build
	[ -e "$script_dir/dist/solc.js" ] || (cd "$script_dir" && npm run build)

[chriseth]

or maybe fail instead of building transparently?

[cameel]

Ah, true. I only tested it quickly with --version and missed that completely :) Fixed.

I think that building transparently is better because the whole point of this is to keep things backwards-compatible. If you can adjust your workflow to run npm build, you can just as well switch to dist/solc.js. I added a comment to make this clear.

[chriseth]

If you do it like that, the change will persist until after the script has run, which is also weird.
What about

Suggested change

	script_dir="$(cd "$(dirname "$0")" && pwd)"
	cd "$script_dir"
	[ -e dist/solc.js ] || npm run build
	exec dist/solc.js "$@" < /dev/stdin
	script_dir="$(cd "$(dirname "$0")" && pwd)"
	(
	cd "$script_dir"
	[ -e dist/solc.js ] || npm run build
	exec dist/solc.js "$@" < /dev/stdin
	)

(not sure if we have to repeat the set comamand)

[cameel]

Closing since I was busy with other tasks and we ended up releasing without it. I think that there's not much point adding it now.