damianavila / RISE

RISE: "Live" Reveal.js Jupyter/IPython Slideshow Extension
Other
3.67k stars 414 forks source link

dev env on windows #582

Open parmentelat opened 3 years ago

parmentelat commented 3 years ago

581 went a little off track and ventured onto the question of having a devel env working on windows

here's the place I propose we continue with this discussion

parmentelat commented 3 years ago

what I just did as a POC

. created a windows virtualbox with my devel repo as a shared disk . as I mentioned, installed gitforwindows . created a miniconda and activated it . added nodejs to the mix with conda . tweaked the package.json in rise-reveal - just one from now, it's a POC - so it can run from the bash in windows

I have pushed this in new branch devel-windows

@kurtmckee can you please let me know if that's something that may be workable for you at all, and if so give it a try in that first folder; if things run fine I can easily propagate the change to the other folders in the repo

kurtmckee commented 3 years ago

@parmentelat, thank you so much for creating that branch! I'll try it out after work and report back.

kurtmckee commented 3 years ago

@parmentelat, thank you again for creating the branch! By copying your bash -c technique to the classic/package.json file, I was able to follow the instructions in develop.md and successfully build and install RISE (with the notable exception that the --symlink option doesn't work).

I didn't need conda or gitforwindows, I just needed to run the "build" commands in git bash, which I already had installed. After the build phase, I was able to run the "install" commands in Powershell.

Now that I can at least build RISE, I'd like to take a swing at creating a GitHub Action so the build steps can be tested on Windows, Linux, and Mac hosts automatically. What is a reasonable assertion that would confirm that RISE can be built correctly on a platform? Could I test for the existence of specific artifacts, like if I run python setup.py sdist in the classic/ directory, could I simply confirm the existence of the dist/*.tar.gz file and verify its contents for specific files?

parmentelat commented 3 years ago

good to know that this angle works; indeed the only requirement is to have bash in one's PATH, I just mentioned gitforwindows as an easy way to fulfil this requirement

would you be so kind as to file a PR with your working classic/package.json so we can merge all this in master ? I guess then a few changes in the devel docs would be in order as well some time, to outline the Windows-specific business: bash is needed (and hints on how to get that right), --symlink won't work, etc... :)

in terms of assertions, I don't feel like I can spell out anything useful off the top of my head without digging in the code more deeply; my angle would be to start from the (updated) devel doc, and run the different steps mentioned in there, e.g.

I am less clear about later steps; but if you take care of setting up the roots, we would be able to add to that on an opportunistic basis

thanks !

kurtmckee commented 3 years ago

@parmentelat, I'll work to prep a PR. If you and @damianavila approve of these goals, then I'll meet these targets:

damianavila commented 3 years ago

That sounds good to me! Btw, I am an occasional Windows user on my Surface (travel laptop)... but I am using WSL-1 stuff... I guess you want a truly Win dev experience and I am OK with that as well if that is the case (just mentioning something I really like and sometimes use, such as WSL).

parmentelat commented 3 years ago

lgtm too I guess we just need to mention that bash is a requirement, and mention gitforwindows as an effective/lightweight way to fulfil that requirement on Windows - in addition to providing git, that is :)

kurtmckee commented 3 years ago

Great! I'll get to work on this. I'm really looking forward to this work! =)

kurtmckee commented 3 years ago

I've pushed a windows-dev branch to my repo. The only things required now are git (in place of patch) and Python >=3.4 (in place of bash, sed, and grep).

I'm now able to run the npm run build steps for both rise-reveal and classic, in both Powershell and git bash. That's a step in the right direction, but I'd appreciate feedback whether it's still working in your environments, too.

Please note that I'm still working toward adding testing to RISE. This branch introduces preliminary tox testing that can be run locally and will be run on Github Actions. Currently only the docs are being tested, since I haven't previously been able to build RISE.

I have not updated the dev docs yet.

I'm eager for feedback! Hope you both had a great weekend!

parmentelat commented 3 years ago

Hi Kurt

as an early feedback here's what I get when trying your branch on my mac; not quite sure what's going on though..

npm run build

> rise-reveal@390.0.1 build /Users/tparment/git/rise/rise-reveal
> python script.py build

patching export/reveal.js/css/reveal.css for RISE
patching export/reveal.js/plugin/notes/notes.js for RISE
patching export/reveal.js/plugin/notes/notes.html for RISE
patching theme export/reveal.js/css/theme/black.css for RISE
patching theme export/reveal.js/css/theme/blood.css for RISE
patching theme export/reveal.js/css/theme/beige.css for RISE
patching theme export/reveal.js/css/theme/serif.css for RISE
patching theme export/reveal.js/css/theme/night.css for RISE
patching theme export/reveal.js/css/theme/white.css for RISE
patching theme export/reveal.js/css/theme/solarized.css for RISE
patching theme export/reveal.js/css/theme/moon.css for RISE
patching theme export/reveal.js/css/theme/simple.css for RISE
patching theme export/reveal.js/css/theme/league.css for RISE
patching theme export/reveal.js/css/theme/sky.css for RISE
patching file export/reveal.js-chalkboard/chalkboard.js for RISE
error: patch failed: rise-reveal/export/reveal.js-chalkboard/chalkboard.js:152
error: rise-reveal/export/reveal.js-chalkboard/chalkboard.js: patch does not apply
Traceback (most recent call last):
  File "script.py", line 195, in <module>
    globals()[arg.replace('-', '_')]()
  File "script.py", line 19, in wrapped
    return function(*args, **kwargs)
  File "script.py", line 170, in build
    patch()
  File "script.py", line 19, in wrapped
    return function(*args, **kwargs)
  File "script.py", line 180, in patch
    patch_chalkboard()
  File "script.py", line 19, in wrapped
    return function(*args, **kwargs)
  File "script.py", line 158, in patch_chalkboard
    subprocess.check_output([
  File "/Users/tparment/miniconda3/envs/rise/lib/python3.8/subprocess.py", line 411, in check_output
    return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
  File "/Users/tparment/miniconda3/envs/rise/lib/python3.8/subprocess.py", line 512, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['git', 'apply', '--unsafe-paths', '--directory=export/reveal.js-chalkboard/', PosixPath('chalkboard.js.patch')]' returned non-zero exit status 1.
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! rise-reveal@390.0.1 build: `python script.py build`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the rise-reveal@390.0.1 build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/tparment/.npm/_logs/2021-02-08T11_59_38_708Z-debug.log
kurtmckee commented 3 years ago

@parmentelat, thanks so much for testing! I'll work to get this fixed.

My first guess is that this is the only code that doesn't use absolute paths so it might be an issue with the current working directory that git runs in; I encountered a similar problem during development and added the --directory to resolve this, but the code can surely be improved regardless of whether it's the root cause of the error you're seeing.

Are you running npm run build while sitting in the rise-reveal/ subdirectory? And, would you run these commands in the rise-reveal/ directory:

pwd
git --version
python -VV
node -v
npm -v

npm run clean
npm install
npm ls | grep reveal

npm run build

git apply --unsafe-paths --directory=export/reveal.js-chalkboard/ chalkboard.js.patch

and share the output with me? I'd also like to review the following files as attachments to this ticket, to ensure that line endings are not normalized by copying-pasting:

kurtmckee commented 3 years ago

@parmentelat, I discovered that trailing whitespace was stripped from the ends of blank lines in the chalkboard.js patch file. It's possible that git on my machine isn't complaining due to a difference in how git apply handles whitespace errors in context lines (like --ignore-whitespace), but please try the branch with the additional commit, and if that still fails please send me the output / attachments listed above.

I'd like to make sure that this is working on your machine before moving forward.

parmentelat commented 3 years ago

hi Kurt things seem to work much better this time

thanks !

in rise-reveal

+ npm run clean

> rise-reveal@390.0.1 clean
> python script.py clean

+ npm install

added 3 packages, and audited 437 packages in 3s

2 packages are looking for funding
  run `npm fund` for details

3 low severity vulnerabilities

To address all issues, run:
  npm audit fix

Run `npm audit` for details.
+ npm ls
+ grep reveal
rise-reveal@390.0.1 /Users/tparment/git/rise/rise-reveal
├── reveal.js-plugins@ (git+ssh://git@github.com/rajgoel/reveal.js-plugins.git#9c128365704334085986d160a45402cfe718223a)
└── reveal.js@3.9.2
+ npm run build

> rise-reveal@390.0.1 build
> python script.py build

patching export/reveal.js/css/reveal.css for RISE
patching export/reveal.js/plugin/notes/notes.js for RISE
patching export/reveal.js/plugin/notes/notes.html for RISE
patching theme export/reveal.js/css/theme/black.css for RISE
patching theme export/reveal.js/css/theme/blood.css for RISE
patching theme export/reveal.js/css/theme/beige.css for RISE
patching theme export/reveal.js/css/theme/serif.css for RISE
patching theme export/reveal.js/css/theme/night.css for RISE
patching theme export/reveal.js/css/theme/white.css for RISE
patching theme export/reveal.js/css/theme/solarized.css for RISE
patching theme export/reveal.js/css/theme/moon.css for RISE
patching theme export/reveal.js/css/theme/simple.css for RISE
patching theme export/reveal.js/css/theme/league.css for RISE
patching theme export/reveal.js/css/theme/sky.css for RISE
patching file export/reveal.js-chalkboard/chalkboard.js for RISE

in classic

+ npm run clean

> rise@5.7.1 clean
> python script.py clean

+ npm install

added 87 packages, and audited 520 packages in 2s

14 vulnerabilities (7 low, 5 moderate, 2 high)

To address issues that do not require attention, run:
  npm audit fix

Some issues need review, and may require choosing
a different dependency.

Run `npm audit` for details.
+ npm ls
+ grep reveal
├── rise-reveal@390.0.1 -> /Users/tparment/git/rise/rise-reveal
+ npm run build

> rise@5.7.1 build
> python script.py less install-rise-reveal
kurtmckee commented 3 years ago

@parmentelat, that's great news! Thank you very much for testing, and I'm pleased it works on Mac, too!

I'll continue to work on meeting the stated targets and then submit a PR.

parmentelat commented 3 years ago

for the record, I was using the windows-dev branch on 8428bb6 on my mac, and ran into this error below it was not a clean repo, I had been working in here before, so this looks like a small glitch in this kind of situation ?

kurtmckee commented 3 years ago

I think I'm overlooking or missing something. Were you referring to the output you previously pasted in? Is there additional output?

On February 21, 2021 11:57:46 AM UTC, parmentelat notifications@github.com wrote:

for the record, I was using the windows-dev branch on 8428bb6 on my mac, and ran into this error below it was not a clean repo, I had been working in here before, so this looks like a small glitch in this kind of situation ?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/damianavila/RISE/issues/582#issuecomment-782845936

parmentelat commented 3 years ago

oops, I'm sorry, I obviously screwed up and failed to copy-paste the output, too bad... no huge deal really, I was just very quickly trying to assess a bug report or something, and fell into that; I'll make sure to report back here if/when it happens again sorry for the noise..

kurtmckee commented 3 years ago

No problem. I've been busy on other fun OSS work so I'm glad that created sufficient time for you to encounter a bug, haha!

I expect I'll return to this later this week. =)