OSVVM / OSVVM-Scripts

OSVVM project simulation scripts. Scripts are tedious. These scripts simplify the steps to compile your project for simulation
Other
8 stars 13 forks source link

Paths for YAML and HTML output files #37

Closed riedel-ferringer closed 1 year ago

riedel-ferringer commented 1 year ago

In OsvvmRequiredSettings.tcl, you define three variables

variable VhdlReportsDirectory     ""
variable OsvvmYamlResultsFile     "OsvvmRun.yml"
variable TranscriptYamlFile       "OSVVM_transcript.yml"

I think this is not entirely correct, because the two yml files are missing the Directory-Prefix. I think this might be correct:

variable VhdlReportsDirectory     ""
variable OsvvmYamlResultsFile     [file join $VhdlReportsDirectory "OsvvmRun.yml"]
variable TranscriptYamlFile       [file join $VhdlReportsDirectory "OSVVM_transcript.yml"]

As VhdlReportsDirectory is set to "", this issue doesn't really matter. However, once you change VhdlReportsDirectory to something else, yml and html output doesn't work any more, because those files are not found.

riedel-ferringer commented 1 year ago

Having said that, one could think about moving VhdlReportsDirectory to the user-modifiable scritps :-)

JimLewis commented 1 year ago

Quick Answer:
file join cannot be done there because CurrentSimulationDirectory is not determined at this point in time. You can enter the simulator, cd, and then start a build. In that case, the build is referenced to the directory in which the build was started.

JimLewis commented 1 year ago

Additional Musings about this issue: The "Required Settings" is for items that if you change, bad things happen.
VhdlReportsDirectory is used by both the scripts and VHDL code for temporary files. Changing it would require changing both the scripts and the VHDL code.

Currently the files created in VhdlReportsDirectory are strictly temporary and only stay there if there is a crash (tool or OSVVM script) that circumvents the scripts from processing the files. In those cases, having them in CurrentSimulationDirectory makes them easy to find.

Currently the only valid value for VhdlReportsDirectory is "". This could change in the future, but we would need a use case that shows having temporary files created somewhere else is a good thing.

It is likely that simulator transcripts (log files) will start as OsvvmRun.log in the VhdlReportsDirectory / CurrentSimulationDirectory. This would simplify addressing issues with log file creation in some simulators - such as running vsim -batch on windows/MSYS2:
vsim -batch |& tee OsvvmRun.log

JimLewis commented 1 year ago

Is there something you are trying to accomplish here that I am missing? If yes, please reopen the issue - I am closing it for now.

riedel-ferringer commented 1 year ago

Additional Musings about this issue: The "Required Settings" is for items that if you change, bad things happen. VhdlReportsDirectory is used by both the scripts and VHDL code for temporary files. Changing it would require changing both the scripts and the VHDL code.

I know :-)

Currently the files created in VhdlReportsDirectory are strictly temporary and only stay there if there is a crash (tool or OSVVM script) that circumvents the scripts from processing the files. In those cases, having them in CurrentSimulationDirectory makes them easy to find.

That's exaclty what I want to avoid. I thought it to be a better solution to place all files, temporary or not, in the same "subfolder" so that my main simdir is not affected. This is not a big deal - I was just playing around. I also changed the VHDL code so that the yaml files are generated accordingly.

Currently the only valid value for VhdlReportsDirectory is "". This could change in the future, but we would need a use case that shows having temporary files created somewhere else is a good thing.

I understand that. But I found that since you do have the defined variable, OsvvmYamlResultsFile and TranscriptYamlFile should be located inside it. I'd even suggest to add a corresponding constant to the VHDL code so that a user could, potentially, change both the tcl and vhdl files to place the output whereever he wants. As I said, it's not a big deal - but I think those [file join] statements are missing anyway :-)

It is likely that simulator transcripts (log files) will start as OsvvmRun.log in the VhdlReportsDirectory / CurrentSimulationDirectory. This would simplify addressing issues with log file creation in some simulators - such as running vsim -batch on windows/MSYS2: vsim -batch |& tee OsvvmRun.log

JimLewis commented 1 year ago

What else do you put in your simdir?

I treat simdir as a temporary. The issue I have is that simulators put temporary files in there. Hence, before starting a regression, I delete the entire directory and recreate it.

That said, it would not be too much trouble to do what you requested. In addition, before setting VhdlReportsDirectory in the required settings, I can check to see if it is set or not - and only set it if it is not already set. That way you can set it in your local defaults. However that said, OSVVM will not recommend that others change it.

riedel-ferringer commented 1 year ago

I was unclear with the term 'simdir': I just mean the folder with the test harness, the test_ctrl entity, the testcases and the .pro files. And i would like to keep it clean.

But don't get me wrong here - this is not a feature request. I just found that as VhdlReportsDirectory exists, it should be part of those other two variables as well. It seemed to me like a "minor bug" for the improbable and not-intended case where someone changes the default value of VhdlReportsDirectory (or in case you will decide to make it configurable in the future)

JimLewis commented 1 year ago

I was using it as:

That's exaclty what I want to avoid. I thought it to be a better solution to place all files, temporary or not, in the same "subfolder" so that my main simdir is not affected.

My simdir is a separate directory that only has simulator temporaries in it. I usually package the scripts with each IP block in the design. So they are either in the VHDL src directory or one level up if the IP comes with both a src and testbench directory. That way the scripts are archived and delivered with the source code.

JimLewis commented 1 year ago

Currently OSVVM testbenches (not in any normal OSVVM library), there is an OsvvmTestCommonPkg.vhd that establishes the path to OsvvmLibraries - using the constant OSVVM_PATH_TO_TESTS. In there the constant OSVVM_RESULTS_DIR is also defined.

I hate having things like this in the library. In fact, when people do this, I no longer think of the library as something that is portable.

That said, what if the file were autogenerated and analyzed by the scripts when OsvvmLibrairies/osvvm/osvvm.pro is run. This would establish the relative path to the OsvvmLibraries - I am thinking though the constant name would be changed to OSVVM_LIBRARIES_DIR (from OSVVM_PATH_TO_TESTS). At this time, I think the package would be named OsvvmSettingsPkg. Its name is not important as it would be included in OsvvmContext.

If that was done, then the constant OSVVM_RESULTS_DIR could be a reflection of the settings for VhdlReportsDirectory. With that, VhdlReportsDirectory could be moved to OsvvmDefaultSettings.tcl.

It would be put in the library osvvm.

@riedel-ferringer @Paebbels Comments appreciated

riedel-ferringer commented 1 year ago

I'm not sure if I get all the implications here (well, I am sure that I don't :-) ), but I'll try to comment anyway. I'll try to summarize my point of view, sorry for being repetitive.

So long story short, in my opinion it would be most portable and usable if there is a vhd package which contains some possibly duplicated settings. The "default user" doesn't have to change anyway, the sophisticated user can configure it to his liking.

JimLewis commented 1 year ago

@riedel-ferringer Deja. :)

And the third way, which is now in the dev release is to have a package, here OsvvmScriptSettingsPkg.vhd, whose constants are auto-generated by the scripts when running OsvvmLibraries/osvvm/osvvm.pro. It defines:

  constant OSVVM_HOME_DIRECTORY        : string := "C:/SynthWorks/Dev/_osvvm/OsvvmLibraries" ;
  constant OSVVM_OUTPUT_DIRECTORY      : string := "" ;
  constant OSVVM_BUILD_YAML_FILE       : string := "OsvvmRun.yml" ;
  constant OSVVM_TRANSCRIPT_YAML_FILE  : string := "OSVVM_transcript.yml" ;

Names are not set in stone yet. The osvvm packages have been updated to use these settings.

It is likely that the approach will evolve some before release - specifically I think these will work better as deferred constants. I want to avoid triggering recompile of the whole OSVVM library. There are already safeguards for compilation - specifically, the package will not be updated if the settings have not changed - however, I think deferred constants will help too.

The OSVVM_OUTPUT_DIRECTORY is set to the same value as ::osvvm::OsvvmTemporaryOutputDirectory. This value is set in OsvvmDefaultSettings.tcl and LocalScriptDefaults.tcl. I expect the default setting may be setting it to match ::osvvm::OutputBaseDirectory - for me, this is not the setting that I need. Typically I run with OutputBaseDirectory as "", except, when running GHDL where I set it to "OSVVM". Hence, I need OsvvmTemporaryOutputDirectory to be "" as it needs to be set to the same value for all simulators run out of the same directory (otherwise recompilation will be a problem).

The ultimate goal of course is to always have the generated files in the correct locations - I suspect this got much better after the error handling was added - this allowed the report tools to at least move the files and try to run even after a failure. The only time I see things not getting created properly is when the simulator crashes and leaves files open. I added actions in the error handler to do "quit -sim" in Questa this should help it close the files. If the old files are not closed properly, new ones cannot overwrite them. I am working to address this. When it does not get moved, I consider it an escape and it needs to be recorded as an error so it can be fixed.

riedel-ferringer commented 1 year ago

Thanks for implementing that. I think we can close this issue.

JimLewis commented 1 year ago

Thanks for the push in that direction. I like what the implementation of this has evolved to be.

Did you note that I updated this with the 2023.04 release? Some expressed concern that generating the whole package was causing their repository to indicate it was out of date. So I reimplemented it using deferred constants in a package and only generated a package body and it is in the .gitignore file If for some reason the package body fails to generate, the compile scripts use the default package body.