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

Allow artiq_compile to use an ARTIQ master #2636

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SquidDev
Copy link
Contributor

ARTIQ Pull Request

Description of Changes

Occasionally it is useful to run artiq_compile on a machine other than the ARTIQ master. For instance, I have a local checkout of our experiment repository, and want to compile an experiment to inspect the generated code. While it is possible to copy the device and dataset DBs to my machine, it would be more convenient if I could pull them direct from the ARTIQ master.

This change adds the standard --server/--port flags to artiq_compile. If specified, the remote device and dataset DB will be used instead.

One caveat with this change is that there is no sanity-checking of arguments. If invoked with both --server and --device-db/--dataset-db, the latter will be silently ignored — sadly there doesn't seem a nice way to fix this with Python's argument parsing. I realise the motivation for this is mostly due to me (ab)using artiq_compile as a debugging tool, so happy if you'd rather close this.

Type of Changes

Type
✨ New feature

Steps (Choose relevant, delete irrelevant before submitting)

All Pull Requests

  • Use correct spelling and grammar.
  • Update RELEASE_NOTES.rst if there are noteworthy changes, especially if there are changes to existing APIs.
  • Check the copyright situation of your changes and sign off your patches (git commit --signoff, see copyright).

Code Changes

  • Run flake8 to check code style (follow PEP-8 style). flake8 has issues with parsing Migen/gateware code, ignore as necessary.
  • Test your changes or have someone test them. Mention what was tested and how: Have run artiq_compile both using --server and with a local dataset+device DB.

Documentation Changes

  • Check, test, and update the documentation in doc/. Build documentation (nix build .#artiq-manual-html; nix build .#artiq-manual-pdf) to ensure no errors.

Git Logistics

  • Split your contribution into logically separate changes (git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.
  • Write short & meaningful commit messages. Review each commit for messages (git show). Format:
    topic: description. < 50 characters total.
    
    Longer description. < 70 characters per line
    

Licensing

See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.

@sbourdeauducq
Copy link
Member

If I understand correctly, all this does is use the remote databases on the master, instead of local files. This should be described as such instead of vague "use an ARTIQ master" (which could be even wrongly interpreted as the master doing the compilation).

And from this perspective the command line issue could be fixed by supporting special filenames, for example master:hostname[:port], in --device-db and --dataset-db.

Release notes need to be updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants