Find verilator include dir using pkg-config

[jck]

In case PYMTL_VERILATOR_INCLUDE_DIR isn't set, use pkg-config to try to find the include dir. This approach works if verilator was installed using a package manager(such as apt-get, pacman or brew).

[cbatten]

I thought that pkg-config info was only set if you installed verilator from a package manager, but it turns out it the pkg-config .pc file is part of the basic verilator source distribution:

• http://www.veripool.org/issues/919-Verilator-pkgconfig-support

is set even when we install verilator via stow. So on the BRG servers, this just works:

  % pkg-config --variable=includedir verilator
  /research/brg/install/stow-pkgs/x86_64-centos6/share/verilator/include

This means we don't really need PYMTL_VERILATOR_INCLUDE_DIR even on the BRG servers which will nicely simplify installation. With respect to Derek's comment, maybe we should use:

 if 'PYMTL_VERILATOR_INCLUDE_DIR' in os.environ:
   verilator_include_dir = os.environ['PYMTL_VERILATOR_INCLUDE_DIR']
 else:
   # use pkg-config

That seems simpler. @dmlockhart do you have any suggestions for the "right" way to call pkg-config? check_output is what you used to call verilator itself, and you also used a try/catch block, although maybe we should catch CalledProcessError:

• https://github.com/cornell-brg/pymtl/blob/master/pymtl/tools/translation/verilator_cffi.py#L79-L95

[jck]

Perhaps, we could 'soft' deprecate the environment variable approach:

• If the env var is present, use it
• Else, just use check_output without catching its exceptions. The exception raised by check_output would be sufficient for the user:

Package verilator was not found in the pkg-config search path.
Perhaps you should add the directory containing `verilator.pc'
to the PKG_CONFIG_PATH environment variable
No package 'verilator' found
Traceback (most recent call last):
...
subprocess.CalledProcessError: Command '['pkg-config', '--variable=includedir', 'verilator']' returned non-zero exit status 1

I've updated the PR with this approach.

[cbatten]

Great! We still might want to catch and print out our own error message so that user knows why PyMTL is trying to run verilator. I will take a look and see if we can merge this in soon.

[jck]

In that case, CalledProcessError and FileNotFoundError(if pkg-config isn't installed) need to be caught.

[cbatten]

Great point!

On Mar 20, 2016, at 12:37 p, Keerthan Jaic wrote:

In that case, CalledProcessError and FileNotFoundError(if pkg-config isn't installed) need to be caught.

---

You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/cornell-brg/pymtl/pull/145#issuecomment-198960250

[cbatten]

I tried adding some more detailed error messages and came up with this.

• https://github.com/cornell-brg/pymtl/blob/jck-verilator/pymtl/tools/translation/verilator_cffi.py#L308-L366

I had to catch OSError not FileNotFoundError. @jck if this looks good to you you can either make the same changes here in your branch, or I can just merge them in via the jck-verilator branch.

[dmlockhart]

@cbatten I actually don't know anything about pkg-config, so I'm going to leave it up to you all to determine what the best solution is. Definitely interested in hearing what you come up with!

[jck]

The jck-verilator branch looks good to merge. However, I'd suggest using universal_newlines=True in check_output because that would be one less thing to fix when python3 support is added.

[cbatten]

Ah right. I will probably wait since we will need to make a pass through all of our uses of check_output to add that. Thanks for your feedback!