test: new job to test the XS worker #1649

warner · 2020-08-31T18:52:44Z

Enhance the test-swingset GitHub Actions CI job to also build and test the new xs-vat-worker executable.

warner · 2020-09-01T02:18:54Z

@michaelfig please take a look at the github workflow file and check that my changes aren't going to mess up / get messed up by the cache operations.. I don't really understand when the cache is read/written to. It'd be nice to cache the XS toolkit (the part built by make $(TOOLS)/mcconfig), which is a function of the git submodule we check out (plus the CI host's compiler/etc), but shouldn't depend upon any agoric-sdk source code. But it's not critical to cache it.. rebuilding that stuff takes about two minutes.

But the xs-vat-worker executable should probably not be cached (or at least we need to be careful to rebuild it at the right times). It's a function of a handful of swingset sources, plus the compartmap.json (which has similar dependencies). I think building xs-vat-worker is pretty quick.

@dckc please check what I did to your Makefile (xs-lin.mk) to make the compile steps be more visible. I deleted compartmap.json and added it to .gitignore because it appears to be a generated file, does that sound right? I moved the git-submodule stuff up higher in the process (to the test-all-packages.yml workflow step that does the original git-clone). And the debian/ubuntu apt package name that contains gio.h was different on all the systems I looked at (libglib not libgio), so I updated that too.

dckc · 2020-09-01T02:52:01Z

packages/xs-vat-worker/xs-lin.mk

+.PHONY: all
+
+all:
+	$(MAKE) -f xs-lin.mk build


why $(MAKE) in a subprocess rather than just make all depend on build?

I wanted a place to emit distinctive echo lines just before each step ran (because they were failing as I figured things out); using the normal make dependencies didn't make it clear when the stages were happening. That said, now that it works, I don't strictly need it anymore.

no strong feelings.

dckc · 2020-09-01T02:53:08Z

packages/xs-vat-worker/xs-lin.mk

 install-gio:
-	sudo apt-get -y update && sudo apt-get -y install libgio2.0-dev
+	sudo apt-get -y update && sudo apt-get -y install libglib2.0-dev


I'm surprised that suffices... I think it won't if/when we use sockets from within XS. But I'm OK with it for now.

From what I can tell (https://packages.ubuntu.com/search?keywords=libgio&searchon=names&suite=all&section=all) libgio2.0-dev isn't actually a package. I think libgio and libglib header/.a files are both included in the libglib2.0-dev package.

michaelfig

LGTM, modulo cleaning up the Github action a bit. It really needs to be meticulously tidied for understandability, since there is no such thing as macros in that yaml file. But you did discern the right things to change.

michaelfig · 2020-09-01T03:42:51Z

.github/workflows/test-all-packages.yml

      if: steps.built.outputs.cache-hit != 'true'
    - name: yarn install
      run: yarn install
      if: steps.built.outputs.cache-hit != 'true'
+    - name: install XS dependencies
+      working-directory: ./packages/xs-vat-worker
+      run: yarn install:xs-lin


Please move this clause below the # END-RESTORE-BOILERPLATE.

michaelfig · 2020-09-01T03:43:00Z

.github/workflows/test-all-packages.yml

    - name: yarn build
      run: yarn build
      if: steps.built.outputs.cache-hit != 'true'
+    - name: yarn build-xs-worker
+      working-directory: ./packages/xs-vat-worker
+      run: yarn build:xs-lin


And this one too. (You didn't change the boilerplate that all the tests use except for the with: submodules: 'true'.)

dckc · 2020-09-01T04:01:15Z

@dckc please check what I did to your Makefile (xs-lin.mk) to make the compile steps be more visible.

Maintainability edits look fine (though I haven't tested them or anything like that). But...

I deleted compartmap.json and added it to .gitignore because it appears to be a generated file, does that sound right?

As I said in our earlier discussion, I'm hesitant to rely on the tool that generates it; have you reviewed tools/findmods.js carefully? I didn't have in mind production-quality lights-out operation when I wrote it; I had in mind that a developer would manually check the results, partly by doing a git diff. In particular, I wouldn't be surprised if it fails on unexpected inputs by silently producing an empty or nonsensical output.

I also wanted to check for compatibility with @kriskowal 's endo work by using typescript; I did that and discovered that there is indeed an arbitrary difference in our designs.

https://github.com/dckc/agoric-sdk/commits/endo-types

I had in mind to use #57 to represent the work to finish the work on findmod.js, ideally by letting endo subsume it.

dckc · 2020-09-01T04:20:18Z

@dckc please check what I did to your Makefile (xs-lin.mk) to make the compile steps be more visible.

I looked over the ci logs and they are as I would expect. Rather satisfying, in fact.

https://github.com/Agoric/agoric-sdk/runs/1054099878?check_suite_focus=true

I somewhat regret that the burden of building this ci step fell to you instead of me, but I suppose it serves as a form of code-review of the stuff I did.

The C code in our branch of the moddable SDK has some debugging diagnostics that I think I originally inherited some from prototyping work that you did; I suppose they should be hidden by default before too much longer but they're OK for now?

lin_xs_cli: loading top-level main.js
 lin_xs_cli: loaded
lin_xs_cli: invoking main(argv)
 lin_xs_cli: main(): entering event loop

dckc · 2020-09-01T04:34:38Z

oh... the .bin path in clean is left-over from earlier; I suppose it's harmless, but while we're doing maintainability edits, maybe get rid of it?

clean:
	rm -rf build $(NODE_MODULES)/.bin/xs-vat-worker

This changes the "test swingset" CI job to additionally build the XS toolkit (written in C), and then use that toolkit to compile the `xs-vat-worker` program. The SwingSet unit tests will then exercise this program (they skip the test unless the program is available). To make the results more visible, another small step was added to run just the one unit test that exercises `xs-vat-worker`. The `git clone` steps were changed to include submodules, since the xs-vat-worker package uses a git submodule to fetch our modified version of the XS source tree. refs #1299

The original name came from the XS build tool convention, but required annoying `-f` flags every time you use it, and we're only ever building for linux/unix here.

warner added the SwingSet package: SwingSet label Aug 31, 2020

warner marked this pull request as draft August 31, 2020 18:52

warner force-pushed the 1299-xs-worker-ci branch 2 times, most recently from 8893b2e to f832a4e Compare September 1, 2020 01:58

warner marked this pull request as ready for review September 1, 2020 02:13

warner requested a review from michaelfig September 1, 2020 02:19

dckc reviewed Sep 1, 2020

View reviewed changes

michaelfig approved these changes Sep 1, 2020

View reviewed changes

warner force-pushed the 1299-xs-worker-ci branch 5 times, most recently from 64306a8 to 0396ced Compare September 21, 2020 21:26

warner changed the base branch from master to 1775-protocol-fix September 21, 2020 21:26

warner force-pushed the 1299-xs-worker-ci branch from 0396ced to 102bf14 Compare October 1, 2020 20:02

warner force-pushed the 1775-protocol-fix branch 2 times, most recently from a077819 to 38c63fb Compare October 1, 2020 20:20

warner force-pushed the 1299-xs-worker-ci branch from 102bf14 to 7324955 Compare October 1, 2020 20:23

Base automatically changed from 1775-protocol-fix to master October 1, 2020 20:33

warner added 6 commits October 1, 2020 13:33

fix: hack on makefile

cb766e7

test: rearrange workflow sections to align BOILERPLATE clauses

edbc346

test: fix yarn command name

42fdc94

chore: rename xs-vat-worker Makefile

9454712

The original name came from the XS build tool convention, but required annoying `-f` flags every time you use it, and we're only ever building for linux/unix here.

fix: xs-vat-worker/Makefile 'clean': there is no .bin/xs-vat-worker

db5e305

warner force-pushed the 1299-xs-worker-ci branch from e621485 to db5e305 Compare October 1, 2020 20:34

warner merged commit 7ff75e1 into master Oct 1, 2020

warner deleted the 1299-xs-worker-ci branch October 1, 2020 20:44

dckc mentioned this pull request Oct 27, 2020

integrate XS into SES-shim ci build / test endojs/endo#508

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: new job to test the XS worker #1649

test: new job to test the XS worker #1649

warner commented Aug 31, 2020 •

edited

Loading

warner commented Sep 1, 2020

dckc Sep 1, 2020

warner Sep 1, 2020

dckc Sep 1, 2020

dckc Sep 1, 2020

warner Sep 1, 2020

michaelfig left a comment

michaelfig Sep 1, 2020

michaelfig Sep 1, 2020

dckc commented Sep 1, 2020 •

edited

Loading

dckc commented Sep 1, 2020 •

edited

Loading

dckc commented Sep 1, 2020

test: new job to test the XS worker #1649

test: new job to test the XS worker #1649

Conversation

warner commented Aug 31, 2020 • edited Loading

warner commented Sep 1, 2020

dckc Sep 1, 2020

Choose a reason for hiding this comment

warner Sep 1, 2020

Choose a reason for hiding this comment

dckc Sep 1, 2020

Choose a reason for hiding this comment

dckc Sep 1, 2020

Choose a reason for hiding this comment

warner Sep 1, 2020

Choose a reason for hiding this comment

michaelfig left a comment

Choose a reason for hiding this comment

michaelfig Sep 1, 2020

Choose a reason for hiding this comment

michaelfig Sep 1, 2020

Choose a reason for hiding this comment

dckc commented Sep 1, 2020 • edited Loading

dckc commented Sep 1, 2020 • edited Loading

dckc commented Sep 1, 2020

warner commented Aug 31, 2020 •

edited

Loading

dckc commented Sep 1, 2020 •

edited

Loading

dckc commented Sep 1, 2020 •

edited

Loading