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

Create shim to run deno bids-validator #1942

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yarikoptic
Copy link
Contributor

This way anyone interested to use it could just

git clone --depth 1 -b deno-build http://github.com/bids-standard/bids-validator
export PATH="$PWD/bids-validator:$PATH"

and start using it and know which version it is:

❯ bids-validator -V
v1.14.6-dev.0-9-gdc61d0f3.deno

An alternative for shim would be just to create it in that branch and then remove in CI only the .js files. I would be happy to redo if feels more sensible.

This way anyone interested to use it could just

    git clone --depth 1 -b deno-build http://github.com/bids-standard/bids-validator
    export PATH="$PWD/bids-validator:$PATH"

and start using it and know which version it is:

    ❯ bids-validator -V
    v1.14.6-dev.0-9-gdc61d0f3.deno

An alternative for shim would be just to create it in that branch and then
remove in CI only the .js files.  I would be happy to redo if feels more sensible.
@effigies
Copy link
Collaborator

This is fine with me. In general, it would be good to get the version into the builds. Running from deno.land also shows "alpha".

Copy link

codecov bot commented Apr 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.86%. Comparing base (bd30213) to head (00418fe).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1942      +/-   ##
==========================================
+ Coverage   85.68%   86.86%   +1.18%     
==========================================
  Files          91      132      +41     
  Lines        3792     6351    +2559     
  Branches     1220     1524     +304     
==========================================
+ Hits         3249     5517    +2268     
- Misses        457      743     +286     
- Partials       86       91       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yarikoptic
Copy link
Contributor Author

yes - this is a poor man solution. Should remain working even when alpha would be replaced with a correct one! note that I added .deno at the end to tell them apart.

@effigies
Copy link
Collaborator

effigies commented Jun 6, 2024

I think this should be superseded by #1985. LMK what you think.

@yarikoptic
Copy link
Contributor Author

for version -- most likely. But there is no shim there, right? in any case -- feel welcome to replace with any proper solution you see fit.

Copy link
Collaborator

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the versioning bits.

.github/workflows/deno_tests.yml Outdated Show resolved Hide resolved
.github/workflows/deno_tests.yml Outdated Show resolved Hide resolved
@effigies
Copy link
Collaborator

Still not sure why the shim is necessary. This:

git clone --depth 1 -b deno-build https://github.com/bids-standard/bids-validator
export PATH="$PWD/bids-validator:$PATH"
bids-validator

doesn't seem significantly less work than:

deno run -A https://github.com/bids-standard/bids-validator/raw/deno-build/bids-validator.js

or:

deno run -A https://github.com/bids-standard/bids-validator/raw/master/bids-validator/src/bids-validator.js

@yarikoptic
Copy link
Contributor Author

Because then you need to know dark inner magic how to run it! And if you need to know that for each and every tool -- life becomes exponentially hard(er).
That is why we have shebangs etc which give user convenience to hide away implementation detail and just be able to "run" it. Sure thing there would be usecases for those alternative invocation schemes (deno-ignorant me will try to remember that trick of deno run -A)

@effigies
Copy link
Collaborator

I believe deno install will do what you want:

deno install -g -A https://raw.githubusercontent.com/bids-standard/bids-validator/master/bids-validator/src/bids-validator.ts
✅ Successfully installed bids-validator
/home/chris/.deno/bin/bids-validatorcat .deno/bin/bids-validator 
#!/bin/sh
# generated by deno install
exec deno run --allow-all --no-config 'https://raw.githubusercontent.com/bids-standard/bids-validator/master/bids-validator/src/bids-validator.ts' "$@"

Or you can compile into a self-contained executable:

$ deno compile -A https://raw.githubusercontent.com/bids-standard/bids-validator/master/bids-validator/src/bids-validator.ts -o ~/.local/bin/bids-validator

@yarikoptic
Copy link
Contributor Author

so install command installs a script which requires download from the web? IMHO odd and undesired -- we should have that script locally and run it.

deno compile sounded interesting but did not produce that bids-validator-deno file:

❯ deno compile -A https://raw.githubusercontent.com/bids-standard/bids-validator/master/bids-validator/src/bids-validator.ts -o /tmp/bids-validator-deno
Compile https://raw.githubusercontent.com/bids-standard/bids-validator/master/bids-validator/src/bids-validator.ts to bids-validator
Archive:  /home/yoh/.tmp/.tmpmsROj6/denort.zip
  inflating: denort                  
❯ cat /tmp/bids-validator-deno
cat: /tmp/bids-validator-deno: No such file or directory
❯ ls -ld /tmp/bids-validator-deno*
ls: cannot access '/tmp/bids-validator-deno*': No such file or directory
❯ deno --version
deno 1.42.4 (release, x86_64-unknown-linux-gnu)
v8 12.3.219.9
typescript 5.4.3

overall -- if there is a standard way to "install", I am all for it I guess for installations. shim is also handy for using it during development while overloading some other (older) bids-validator as we did in

https://github.com/bids-standard/bids-examples/blob/master/.github/workflows/validate_datasets.yml#L76

@yarikoptic yarikoptic changed the title Add version into deno bids-validator.js + create shim to run it Create shim to run deno bids-validator Jun 10, 2024
@effigies
Copy link
Collaborator

effigies commented Jun 11, 2024

so install command installs a script which requires download from the web? IMHO odd and undesired-- we should have that script locally and run it.

deno install -g $PERMISSIONS $URL

If $URL is a local directory, it will work exactly how you are proposing with your shim:

git clone --depth 1 -b deno-build http://github.com/bids-standard/bids-validator
deno install -g -A bids-validator/bids-validator.js

If you want to install from deno.land (latest tag):

deno install -g -A https://deno.land/x/bids_validator

If you want to install from GitHub:

deno install -g -A https://github.com/bids-standard/bids-validator/raw/$REF/bids-validator/src/bids-validator.ts

Deno caches things locally, so you shouldn't hit the internet after the first run unless you clear the cache.

deno compile sounded interesting but did not produce that bids-validator-deno file:

The -o $TARGET needs to come before the thing you're compiling it looks like.

deno compile -o /tmp/bids-validator-deno -A https://raw.githubusercontent.com/bids-standard/bids-validator/master/bids-validator/src/bids-validator.ts

@yarikoptic
Copy link
Contributor Author

wow -- it even compiles a full fledged executable!

❯ ldd /tmp/bids-validator-deno
	linux-vdso.so.1 (0x00007fff7c334000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f49bdf8a000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f49bdf5d000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f49bdf58000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f49bde76000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f49bdc91000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f49c2c97000)
❯ file /tmp/bids-validator-deno
/tmp/bids-validator-deno: ELF 64-bit LSB pie executable, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 2.6.32, BuildID[xxHash]=1306fac976605c34, stripped
❯ ls -ld /tmp/bids-validator-deno
-rwxrwxrwx 1 yoh yoh 82058405 Jun 11 09:45 /tmp/bids-validator-deno*

that's crazy! All great info -- worth adding to some README.md somewhere, and may be in particular in that deno-build branch.

I will leave it up to you -- I would still have preferred to have a helper shim so I do not need to wonder what to do. But RTFM approach would also suffice -- at least it would make it easy for people to do what they want to do.
May be even under bin/ subfolder as deno install --root . -A bids-validator.js does, but without hardcoded path as it does.

@yarikoptic
Copy link
Contributor Author

with that binary build, I even wonder where/how could we stick it within deno-build -- having usable bids-validator without all the deno stack would be great! At least it should become part of the releases distributions IMHO.

@yarikoptic
Copy link
Contributor Author

oh, that was just a naive dream -- that executable is not self contained etc (obviously needs bundle of schema etc anyways) and also depends on hardcoded paths...

bids@rolando:~$ ./bids-validator-deno --version                                                                                                                 
fatal: cannot change to '/tmp/bids-validator/bids-validator/src': No such file or directory
bids-validator file:///tmp/bids-validator/bids-validator/src/bids-validator.ts

but it works if I provide a clone there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants