vega / editor

Editor/IDE for Vega and Vega-Lite
https://vega.github.io/editor/
BSD 3-Clause "New" or "Revised" License
165 stars 89 forks source link

Setup instructions don't work on windows #594

Closed abzachshan closed 4 years ago

abzachshan commented 4 years ago

editor\src\constant\spec.ts requires 2 files:

export const VEGA_SPECS = require('../../public/spec/vega/index.json'); export const VEGA_LITE_SPECS = require('../../public/spec/vega-lite/index.json');

I followed the rather simple instructions in readme to start up the project but because both of the specs are non-existent in the project, it causes a compilation failure (See 1st image).

These files were deleted in a previous commit and I was able to get it started when I pulled them from the commit and put them in the correct folders - I imagine this is not how it is supposed to be and there's a reason for their removal.

There's a problem using the docker-compose as-well (See 2nd image). I don't know if the cause is the same.

Would love to get some help.

Untitled

Untitled1

domoritz commented 4 years ago

I don't have a windows machine and it looks like the instructions don't work there. Can you help fix it?

rav1kantsingh commented 4 years ago

I had the same issue on mac as in your first image your specs folder would be empty. In my case, the issue was with wget. After installing wget everything worked. https://github.com/vega/editor/issues/495

abzachshan commented 4 years ago

Hey @domoritz and @IamRavikantSingh, I was able to narrow down the problem - the scripts that are used (located in 'editor/scripts') when you run the yarn command, are heavily reliant on Linux OS commands which are non-existent on Windows OS (such as wget, rsync, tar).

I'll try working on a generic solution whenever I can

domoritz commented 4 years ago

I see. Rather than making the script work on windows, let's just have people use docker instead and make sure docker works, okay?

abzachshan commented 4 years ago

Sure. I believe fixing the docker and publishing the dockerized image would be a great idea! I was also hoping there would be one when we first tried to set this up. I will try to look into it

domoritz commented 4 years ago

I don't think we need to publish an image. Just make the setup we have work.

baumanab commented 4 years ago

This is mostly in reference to the dockerfile..Based on my experiments it looks like the issue is the line endings on 'vendor.sh' and 'version.sh' when cloning into windows. The current sed lines are not fixing this for some reason, even after I apply them to both files. If I remove lines after copy .. in the docker file, run the container, and convert crlf to lf (using vim) and then run yarn and yarn start everything works. I'm working out a more elegant solution.

abzachshan commented 4 years ago

@baumanab I have a pull request fixing the dockerfile and cleaning it a bit - I’d appreciate if you could take a look

baumanab commented 4 years ago

I didn't find a more elegant solution. Here is something that works though.

docker build -t vle .

If you are on window it's safest to run this next one in powershell due to the mount

docker run -dit -p 8080:8080 -v ${PWD}/src:/usr/src/app/src vle

I made some changes to the dockerfile.

FROM node:latest

# Fetch updated list of packages and upgrade operating system for next step (install rsync)
RUN apt-get update && apt-get upgrade -y && \
    apt-get -y install rsync \
      dos2unix \
&& rm -rf /tmp/downloaded_packages/ /tmp/*.rds \
    && rm -rf /var/lib/apt/lists/*

# Sets the working directory for any RUN, CMD, ENTRYPOINT, COPY and ADD instructions that follow it in the Dockerfile
# https://docs.docker.com/engine/reference/builder/#workdir
WORKDIR /usr/src/app

# Copies the package.json and yarn.lock files first to ensure the cache is only invalidated when these files change
# https://nodejs.org/en/docs/guides/nodeodejs-docker-webapp/#creating-a-dockerfile
COPY package.json yarn.lock ./

# For this project, additional files must also be copied as yarn hooks depend on them
COPY scripts ./scripts

# Copy remaining files
COPY . .

# Remove 'r' characters from the vendor script (otherwise it won't execute)
RUN dos2unix ./scripts/vendor.sh && \
    dos2unix ./scripts/version.sh 

# Run Yarn
RUN yarn

# Sets the container executable (ENTRYPOINT) as yarn and the default argument (CMD) as start
# https://docs.docker.com/engine/reference/builder/#entrypoint
# https://docs.docker.com/engine/reference/builder/#cmd
ENTRYPOINT ["yarn"]
CMD ["start"]
baumanab commented 4 years ago

@baumanab I have a pull request fixing the dockerfile and cleaning it a bit - I’d appreciate if you could take a look

I didn't see your message before my post. I'll try it out. If there is any interest in the Dockerfile I used, I can put in a PR as well.

baumanab commented 4 years ago

it works, leaving comments in the PR

domoritz commented 4 years ago

Fixed in master. Use docker.