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

feat: add support for json config files #55

Merged
merged 4 commits into from
Jun 1, 2024
Merged

Conversation

htfab
Copy link
Collaborator

@htfab htfab commented May 28, 2024

The first commit adds a library for reading and writing configuration files in tcl, json and yaml formats. This allows us to merge configuration files, so we no longer need to source user_config.tcl from config.tcl. Sourcing is still supported for compatibility with old config.tcl files, but all variables defined while evaluating config.tcl are overwritten by those appearing in user_config.tcl, creating a config_merged.tcl file that is used in the flow.

The second commit switches the defaults, config.json is now preferred over config.tcl, and the generated files are user_config.json and config_merged.json. We still generate user_config.tcl as well because an old-style config.tcl might still source it. We can remove that once we branch off tt08.

The third commit is a single-line change that also allows config.yaml. I've split it off so that it can be independently decided upon. I personally find config.yaml much more readable, but we introduce some divergence from upstream openlane. The generated files are still kept in json.

After merging the PR tt-support-tools should remain compatible with existing configuration files but also support these:
https://github.com/htfab/tt07-verilog-template/blob/tcl-config/src/config.tcl (doesn't source user_config.tcl)
https://github.com/htfab/tt07-verilog-template/blob/json-config/src/config.json
https://github.com/htfab/tt07-verilog-template/blob/yaml-config/src/config.yaml

@urish
Copy link
Member

urish commented May 28, 2024

Thanks! I've just branched tt08 - let's move this to tt08?

@htfab htfab changed the base branch from tt07 to tt08 May 28, 2024 09:24
@urish
Copy link
Member

urish commented May 29, 2024

We can remove that once we branch off tt08.

Then I guess we can already remove it?

@urish
Copy link
Member

urish commented May 30, 2024

Thanks!

The third commit is a single-line change that also allows config.yaml. I've split it off so that it can be independently decided upon. I personally find config.yaml much more readable, but we introduce some divergence from upstream openlane. The generated files are still kept in json.

I think we need to make a call here - either we go with yaml (and let's talk to openlane devs about this to get their opinion as well), and then use config.yaml as the default, either we only support config.json.

I did a lot of research back in the day for configuration file formats, and unfortunately, each format has some drawbacks:

  1. JSON: most popular, but lacks comments (and other features that may be important for some specific cases, e.g. 64-bit number support, or NaN value support)
  2. JSON with some extensions: JSONC (adds comments and trailing commas) or JSON5 (adds even more improvements, such as single quoted strings) - limited language support / ecosystem
  3. YAML - too many features, with some confusing gotchas (e.g. https://www.bram.us/2022/01/11/yaml-the-norway-problem/ - we have hit it in the past with info.yaml), and also,
  4. INI files - simple, non standardized, non typed (there's a standard for them in Python, but it's not widely adopted by other languages)
  5. TOML - not as widely supported as JSON/YAML, but gaining traction (I think Rust helped popularized it, and now there's even a parser built into Python). IMHO a good middle ground between JSON/YAML. The string semantics are a bit confusing (strings with double quotes support escape sequences, single quotes do not).

So it's hard to make a good call :)

@urish urish merged commit 59c9f8e into TinyTapeout:tt08 Jun 1, 2024
3 checks passed
urish added a commit that referenced this pull request Jun 1, 2024
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