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

OutputManager for files produced by Apalache #1036

Merged
merged 13 commits into from
Oct 22, 2021
Merged

Conversation

Kukovec
Copy link
Collaborator

@Kukovec Kukovec commented Oct 12, 2021

  • Tests added for any new code
  • Ran make fmt-fix (or had formatting run automatically on all files edited)
  • Documentation added for any new functionality
  • Entry added to UNRELEASED.md for any new functionality

Prototype implementation, following #1025.
Closes #977

@Kukovec Kukovec mentioned this pull request Oct 12, 2021
4 tasks
docs/src/apalache/config.md Outdated Show resolved Hide resolved
docs/src/apalache/config.md Outdated Show resolved Hide resolved
docs/src/apalache/config.md Outdated Show resolved Hide resolved
docs/src/apalache/config.md Outdated Show resolved Hide resolved
docs/src/apalache/config.md Outdated Show resolved Hide resolved
mod-tool/src/main/scala/at/forsyte/apalache/tla/Tool.scala Outdated Show resolved Hide resolved
Kukovec and others added 3 commits October 13, 2021 21:53
Co-authored-by: Shon Feder <shon@informal.systems>
Co-authored-by: Shon Feder <shon@informal.systems>
Co-authored-by: Shon Feder <shon@informal.systems>
Copy link
Collaborator

@konnov konnov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's definitely a good improvement over the chaos that we currently have about files and directories. Since we are refactoring now, shall we make one more step to make code nice:

  • introduce a trait that gives us the output manager and configuration options
  • this would require minor refactoring of PassOptions (renaming?).

docs/src/apalache/config.md Outdated Show resolved Hide resolved
docs/src/apalache/config.md Outdated Show resolved Hide resolved
- `OUTDIR`: The value of `OUTDIR` defines a directory, in which all Apalache runs write their outputs. Each run will produce a unique subdirectory inside `OUTDIR` using the following convention: `<SPECNAME>_yyyy-MM-dd_HH-mm-ss_<UNIQUEID>`.
Example value: `~/apalache-out`.
If this value is not defined in `apalache.cfg`, Apalache will, for each individual run, define it to be equal to `DIR/x/`, where `DIR` is the directory containing the specification.
- `profiling`: This Boolean flag has to be one of `TRUE/true/FALSE/false` and governs the creation of `profile-rules.txt` used in [profiling](profiling.md). The file is only created if `profiling` is set to `TRUE/true`. Setting `profiling=false` is incompatible with the `--smtprof` flag.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we avoid these TRUE/true/FALSE/false and use a standard parser for configuration files? Either Java properties file or YAML. I am not fan of yaml, but hand-written configs are even less fun.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, like I said in a previous answer, this was just the fastest way to get the proof-of-concept prototype working, without committing too much.

Comment on lines 233 to 234
TLA IR.
- File `out-parser.json` is produced as a result of converting the Apalache TLA IR representation of the input into JSON
format.
- File `out-config.tla` is produced as a result of substituting CONSTANTS, as described
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happened tou out-parser.json? The MBT team has asked for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The text above says "each pass of the model checker produces an intermediate TLA+ file in the run-specific
    directory"
  2. ALL of the files listed here have .json counterparts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, 3) Rodrigo renamed all these files, so their names are out of sync, but that's a TODO for later

mod-tool/src/main/scala/at/forsyte/apalache/tla/Tool.scala Outdated Show resolved Hide resolved
Comment on lines 50 to 61
def syncFromCFG(): Unit = {
val flagRegex = raw"^\s*([a-zA-Z\-]+)\s*=\s*(.+)\s*$$".r
val home = System.getProperty("user.home")
val configFile = new File(home, CFG_FILE)
if (configFile.exists()) {
val src = Source.fromFile(configFile.getAbsolutePath)
for (line <- src.getLines) {
flagRegex.findAllMatchIn(line).foreach { m =>
// Flags have the shape FLAGNAME=FLAGVAL
val flagname = m.group(1)
val flagVal = m.group(2)
if (flagname == OUTDIR_NAME_IN_CFG) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a prototype, so I just wanted a quick-and-dirty txt file to read

Let's not do that. There are tonnes of libraries around in the Java/Scala world. We do not want to do IO ourselves, unless we really badly need to do that.

Comment on lines 64 to 65
if (flagVal.startsWith("~")) flagVal.replaceFirst("~", home)
else flagVal
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether standard libraries like Apache Commons Configuration would be able to handle ~, perhaps, they would work with environment variables like $HOME. In any case, we should never implement this kind of things, unless we have a really good reason to do it. Check Apache Commons Configuration, or google for a YAML parser.

Copy link
Collaborator

@konnov konnov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Please update the relevant sections of the manual, as the previously available files are not written by default anymore.

@Kukovec
Copy link
Collaborator Author

Kukovec commented Oct 22, 2021

Manual changes are in #1025. This branch merges into the ADR one, not unstable directly.

@Kukovec Kukovec merged commit bcfa68e into jk/outputsADR Oct 22, 2021
@Kukovec Kukovec deleted the jk/outputManager branch October 22, 2021 09:04
Kukovec pushed a commit that referenced this pull request Oct 22, 2021
* Outputs ADR

* tweaks

* Apply suggestions from code review

Co-authored-by: Igor Konnov <igor@informal.systems>

* tweaks

* tweaks

* OutputManager for files produced by Apalache (#1036)

* OutputManager

* logs

* comments

* removed unnecessary file

* tweaks for logless runs

* string.strip nonsense

* docs

* Update docs/src/apalache/config.md

Co-authored-by: Shon Feder <shon@informal.systems>

* Update docs/src/apalache/config.md

Co-authored-by: Shon Feder <shon@informal.systems>

* Update docs/src/apalache/config.md

Co-authored-by: Shon Feder <shon@informal.systems>

* v2, with YAML

* md

* run.txt

Co-authored-by: Shon Feder <shon@informal.systems>

* unreleased

* fmt

Co-authored-by: Igor Konnov <igor@informal.systems>
Co-authored-by: Shon Feder <shon@informal.systems>
This was referenced Oct 25, 2021
@Kukovec Kukovec mentioned this pull request Nov 17, 2021
4 tasks
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.

3 participants