jw3 / example-daffodil-vscode

A VS Code extension for DFDL with Daffodil
Apache License 2.0
2 stars 3 forks source link

Prototype monorepo of Typescript extension with Scala backend. #76

Closed arosien closed 3 years ago

arosien commented 3 years ago

For #37.

jw3 commented 3 years ago

Looking at this I was surprised the source was merged together. Was expecting a separate dirs for scala and typescript.

Is this how other extensions do it?

jw3 commented 3 years ago

In my head it was looking like

root
- dapodil/main/scala/...
- extension/...
arosien commented 3 years ago

Looking at this I was surprised the source was merged together. Was expecting a separate dirs for scala and typescript.

Is this how other extensions do it?

I just literally merged one directory structure onto the others. Typescript projects don't follow the same directory naming schemes as "maven"-style projects use. But at least the source is under src!?

That said, we could put some effort into making it look nicer. I'm just being lazy and trying not to have to learn yet another build tool.

jw3 commented 3 years ago

OK, that sounds fine. Sounds like you don't have any strong opinions on it, and I don't have strong opinions either. It definitely catches the eye and probably will generate questions, so maybe Ill tinker on it some and look at other options.

jw3 commented 3 years ago

Added daffodil version to the artifact name

dev: daffodil-debugger-3.1.0-0.0.8-9-g282ef0c.zip tag: daffodil-debugger-3.1.0-0.1.0.zip

arosien commented 3 years ago

Hi @Shanedell and @jw3, I wanted to put together a list of changes needed in this PR:

jw3 commented 3 years ago

Ill take a look today

arosien commented 3 years ago

Currently fixing https://github.com/jw3/example-daffodil-vscode/pull/76/checks?check_run_id=3365376715.

arosien commented 3 years ago

Currently fixing https://github.com/jw3/example-daffodil-vscode/pull/76/checks?check_run_id=3365376715.

Ah, I see you fixed this @jw3

jw3 commented 3 years ago

Ok, I had bumped the sbt build up on the list so the vscode stuff didnt dirty the working dir and cause a snapshot tag on the backend artifact. Doing that though caused the backend package to get zipped up into the vsix.

So I was thinking of an ignore for the vsix on the sbt artifact, or preventing the vscode side from dirtying the repo (you already gitignore the version file so maybe the npm lock file?)

~If you are looking at already Ill let it be.~

We have a race condition in the comment section....

jw3 commented 3 years ago

Adding vscode ignores for the sbt things fixed the size of the vsix locally. Pushed that up.

arosien commented 3 years ago

The nightly.yml file likely also needs fixing.

jw3 commented 3 years ago

Installed the extension and tried to debug, doesnt look like something along the way likes the -pre suffix.

The extension claims,

Couldn't download the Daffodil debugger backend from https://github.com/jw3/example-daffodil-debug/releases/download/0.0.10/daffodil-debugger-3.1.0-0.0.10.zip.

But the file is https://github.com/jw3/example-daffodil-vscode/releases/download/v0.0.10-pre3/daffodil-debugger-3.1.0-0.0.10-pre3.zip

(I am done here for a bit, will check in later.)

arosien commented 3 years ago

Installed the extension and tried to debug, doesnt look like something along the way likes the -pre suffix.

The extension claims,

Couldn't download the Daffodil debugger backend from https://github.com/jw3/example-daffodil-debug/releases/download/0.0.10/daffodil-debugger-3.1.0-0.0.10.zip.

But the file is https://github.com/jw3/example-daffodil-vscode/releases/download/v0.0.10-pre3/daffodil-debugger-3.1.0-0.0.10-pre3.zip

(I am done here for a bit, will check in later.)

The version in package.json needs to match the tag, so I'm guessing you just made a git tag manually, vs. running npm version. (The version in package.json is what the extension uses.)

arosien commented 3 years ago

@jw3 I fixed the version in package.json and re-released 0.0.10-pre-3 and will test it.

arosien commented 3 years ago

@jw3 I fixed the version in package.json and re-released 0.0.10-pre-3 and will test it.

Hmm the rebuild vsix didn't seem to pick up the new version. If I build it locally I get a different error:

Got:

Couldn't download the Daffodil debugger backend from https://github.com/jw3/example-daffodil-debug/releases/download/0.0.10-pre3/daffodil-debugger-3.1.0-0.0.10-pre3.zip.

This is because the tag has the v prefix, which I think we should remove. Using npm version should create a non-prefixed tag.

arosien commented 3 years ago

However.... we need to ensure the version in package.json matches the version used by the (Scala) build.sbt, which currently reflects off of the current git commit. So if the vsix is released first, and npm version is run, a new git tag will be created and the Scala built should occur next.

shanedell commented 3 years ago

I think I have an idea of how to get the release to work with the tag being used for git.

So we keep exporting the git tag to an ENV but then we run a sed (or something similar) command on the package.json to replace the version: with the tag, without the v. This should allow us not need to make a commit every time we want make a new release it will just be automatically updated through the release action.

shanedell commented 3 years ago

This is what I added to release.yml to make the CI update the package.json based on tag.

- name: Fix version in package.json
   run: |
     sed -i "s|\s\s\"version\":.*|  \"version\": \"${{env.GIT_TAG}}\",|" package.json

This will allow for us to not necessarily need to keep the version tag up to date in the package.json since it will be auto updated.

This tag was built using it https://github.com/jw3/example-daffodil-vscode/releases/tag/v0.0.10-pre4

Also updated the artifact path to add that v to the url, however did not seem to fix that issue will continue looking into it though. Seems like it point to example-daffodil-debug instead of example-daffodil-vscode going to update and test now.

jw3 commented 3 years ago

I didnt quite grok what was happening with the tags earlier. Going to think about this some, but right now I think

So maybe rather than trying to automate the sync, we instead add build checks that guard against an incorrect configuration, using CI kick it back to the responsible human to sync things up.

:thinking:

shanedell commented 3 years ago

That could work as well. I figured we would want to keep the package.json up to date as much as possible but I also thought automating the process of updating the tag in the CI would be helpful.

For example if you are working on a branch and plan to go to release v0.0.11 then you update the package.json to 0.0.11, however before making that release you want to a test one to make sure everything works so you just using this way you can leave the package.json file alone and just make a new tag v0.0.11-pre and it will be updating in CI to that. That was just my thought process behind the automating.

However, right now this should be a good tag to test out:

jw3 commented 3 years ago

Reference https://github.com/jw3/example-daffodil-vscode/issues/83 for details on how the workflow of this project will be once it moves to the new org.

arosien commented 3 years ago

I have a worry that when a tag is added, I'm not sure which GitHub action fires and builds the files. Is it both this PR and the main line? Do they race? Or is it only the main line? I'm trying to explain the vsix file not knowing the correct version.

jw3 commented 3 years ago

Well tagging this PR for testing is a little unusual, the normal process in a real-world scenario would be something like

  1. PR is created that bumps the version
  2. CI workflows and human reviewers all verify versions
  3. PR is merged
  4. Tag is added on master
  5. The release workflow runs and verifies versions match
  6. Release is created at end of release workflow
jw3 commented 3 years ago

The nightly build would have a gap in the scenario where all updates are manual. We probably need a git describe in there to generate the nightly version number.

Probably a good start would be git describe --match "v*"

which will track annotated v tags.

shanedell commented 3 years ago

Code has been updated to remove the auto fixing of the package.json and to instead check if the version in package.json matches the git tag before allowing the builds to happen. If you look at https://github.com/jw3/example-daffodil-vscode/actions/workflows/release.yml latest two fails are from having a version inside of the package.json that did not match the tag pushed. The most recent good one is after the version in package.json to match tag.

arosien commented 3 years ago

I downloaded the -pre6 vsix and it worked for me!

jw3 commented 3 years ago

we instead add build checks that guard against an incorrect configuration

This is now implemented. How you keep the versions in sync and create tags can vary, the fail safe is in the release build which will fail to prevent a bad build from being created.

I also tested -pre6 and had success.

jw3 commented 3 years ago

Merged and tagged v0.0.10. Removed all the pre release tags except for the working one, pre6.

arosien commented 3 years ago

Huzzah! 🎉