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

Portability Issues When Used As Imported Library #23

Open
asmr-hex opened this issue Aug 26, 2018 · 2 comments
Open

Portability Issues When Used As Imported Library #23

asmr-hex opened this issue Aug 26, 2018 · 2 comments
Assignees
Labels
discussion enhancement New feature or request

Comments

@asmr-hex
Copy link
Collaborator

Overview

I've been working on using the xaqt package as in imported library in an application I'm working on and there are a couple difficulties I've run into which seem to require some internal refactoring. The main underlying issue in both issues is related to assumptions made about the locations of asset files and directories within the xaqt package.

data/Payload

this directory contains all the scripts necessary to actually run the submitted user code. on each call of (*sandbox).run() this directory is copied over into a temporary directory which is mounted as a volume in the docker container. when the directory is copied, it assumes that the data/Payload directory is located on the GOPATH, specifically at $GOPATH/src/github.com/frenata/xaqt/data/Payload. While this will be true most of the time, this assumption will not hold if the xaqt package is vendored. I am guessing that this is the most likely case for most deployed applications which want to have a stable build/deploy process (since go get does not yet support (?) getting locked versions of dependencies).

Because of this, I propose that the data/Payload directory is copied into the docker container at image build time rather than during application run time. This will be beneficial in two ways:

  1. it will eliminate the issue described above
  2. it will reduce the amount of work that the application has to do repeatedly at run time (i.e. copy this directory to the docker container on each call of (*sandbox).run()).

compilers.json

this is a similar issue as described above. the calling code at run-time will not be able to resolve the location of this file if it is vendored. Additionally, this file (for some reason) isn't even included in the vendor directory (when using the govendor tool). I propose that for the default compilers.json map included with the library, we include it as a string constant in a .go file such that is it part of the package and not an external asset file. We can still include the json file in the repo s.t. users who wish to use their own compiler mapping/custom docker image can have an example of how to construct this json map.

@frenata
Copy link
Owner

frenata commented Aug 27, 2018

Because of this, I propose that the data/Payload directory is copied into the docker container at image build time rather than during application run time

See #11

this is a similar issue as described above. the calling code at run-time will not be able to resolve the location of this file if it is vendored

Yeah I know this came up in @nefelin 's usage of the library as well. We talked about several "emergency" fixes. I think it's a solid change to pull the json into a source file directly, then provide it as a default for box creation, if the user chooses not to specify their own compilers map. This should be possible both via API and CLI.

@asmr-hex
Copy link
Collaborator Author

@frenata oh nice! sorry i didn't see issue #11 before! Its good to know that we are on the same page about it! I started working on that this morning and will try to push something soon and open a PR.

Additionally, i also put the current compilers.json into the compilers.go source file as a Compilers map type. I also included a ExportToJSON method on the Compilers type and will make sure it runs when the tests are run s.t. any changes to the map will be reflected in the repo (assuming that developers run the tests (and check in the new json file) upon making changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants