search

riscv / configuration-structure

RISC-V Configuration Structure
https://jira.riscv.org/browse/RVG-50
Creative Commons Attribution 4.0 International
36 stars 16 forks source link

Split and organize schema.asn #42

Closed changab closed 3 years ago

changab commented 3 years ago

Split and organize schema.asn into smaller schema files under /schema for easy maintenance. /schema/configuration-structure.asn - Renamed from schema.asn /schema/debug-extension.asn - Debug extension schema /schema/hart-extension.asn - Hart extension schema /schema/helper-types.asn - Other generic types schema

rvcs searches the files under /schema as the standard schema files. "--schema" argument is used for the additional schema other than standard ones. One of use cases is vendors to generate the binary with their schema alone with the standard schema files.

changab commented 3 years ago

I really like splitting out the schema like that. It will make maintenance a lot easier.

I'm not sure about --schema only specifying additional schemas. There ought to be some way to ignore the defaults altogether. I think the simplest is to interpret a missing --schema option to use the defaults, and every other option requiring every single .asn to be listed. That's really not so hard for the user. (E.g. rvcs.py --schema /path/to/schema/* custom.asn.)

I think we can specify not just the additional schemas. We can allow users to specify the schemas and reapalce the data types (with the identical notation of the data type) defined in the default schema with the one declared in the schemas specified in "--schema". However, we don't allow users to disorder the data type sequence or value type defined in the standard schema, they can just have the vendor-defined data type.

We can add a feature later that if a directory name is given to --schema, it automatically finds every asn file under that directory to make this even easier. (Do you need that on Windows? Do modern Windows shells still not do file globbing?)

Sure. This would be much easier for users.

timsifive commented 3 years ago

I think we can specify not just the additional schemas. We can allow users to specify the schemas and reapalce the data types (with the identical notation of the data type) defined in the default schema with the one declared in the schemas specified in "--schema". However, we don't allow users to disorder the data type sequence or value type defined in the standard schema, they can just have the vendor-defined data type.

That all sounds reasonable, but more complex to implement. If you have time for this, that is great. But I'd prefer to quickly get this PR merged. For now can you make the simpler change which defaults to all the usual .asn files, but requires the user to specify every .asn file if they want to do something custom?

changab commented 3 years ago

That all sounds reasonable, but more complex to implement. If you have time for this, that is great. But I'd prefer to quickly get this PR merged. For now can you make the simpler change which defaults to all the usual .asn files, but requires the user to specify every .asn file if they want to do something custom?

Yes, for now, I will split the schema and use the default .asn files under a new folder schema/. Users can specify the .asn file that could be a totally new schema or the overwrite schema to the usual one.

I will address the "commas" issues after the above is committed.

changab commented 3 years ago

rvcs searches the files under /schema as the standard schema files. "--schema" argument is used for the additional schema or overwrite the default schema.

I will investigate how to use "nargs" to avoid the comma of the given schema files against "--schema".

changab commented 3 years ago

@timsifive thanks for those good comments on Python language. Update at https://github.com/riscv/configuration-structure/pull/42/commits/837a755bafa07c22ef450a076204054f19aec029

changab commented 3 years ago

I'm not a fan of schema_list being global, but let's get this merged so we can continue with the other PRs. I'm planning to do a cleanup pass at some point.

Maybe pass to the function or some other ways. Sure, we can clean it up. thanks