draperlaboratory / hope-policies

Collection of security policies
3 stars 2 forks source link

Remove dependency on ISP_PREFIX #16

Open ccasin opened 6 years ago

ccasin commented 6 years ago

Running tests with qemu and the hifive config fails if ISP_PREFIX is not set in the user's environment. All scripts should use "/opt/isp/" as a default when ISP_PREFIX is not set.

Austin - can you see if this is easy to fix?

austinharris commented 6 years ago

It's cumbersome to set the default in all the python scripts across policy tests, gem5, etc. So I have been advocating for requiring the environment variable be set always.

ccasin commented 6 years ago

We used to have several environment variables (including ISP_PREFIX), and it made running all the tools really error prone. Often people would spend hours debugging problems that turned out to be an environment variable that needed to be tweaked. As a result, Eric spent a bunch of time revising the build system so that there are no longer any dependencies on environment variables - I'd really like to keep this property.

austinharris commented 6 years ago

You already have to have this one variable on your path. This is very different than the previous situation.

ccasin commented 6 years ago

All the other tools use a default of "/opt/isp/" and don't require this variable set - that's the desired behavior. We worked hard to eliminate the dependency on ISP_PREFIX. I'll try to find time to fix this later.

austinharris commented 6 years ago

Again, the dependency still exists and it was never fully fixed (these tools existed back then but Eric didn't fix them all). /opt/isp/bin must be on your path, thus the user must be setting up an environment variable already so I really don't see what you gain here except for code clutter and a bunch more places we have to change things to make Dover happy when we remove ISP everywhere. Can you please give justification for why this is the desired behavior?

ccasin commented 6 years ago

This is the only place where the toolchain relies on the ISP_PREFIX envrionment variable. The setup instructions in hope-tools say that as long as you put /opt/isp/bin on your PATH, no other changes are needed and in particular no ISP-specific environment variables are needed. ISP_PREFIX is intended to be an optional way to change from the default /opt/isp/bin location, not a mandatory variable.

Of course, it would be easier to make ISP_PREFIX required than to check for it in all the scripts. We explicitly decided not to do that in an effort to eliminate all ISP-specific environment variables. I don't want to start back down the path of ISP-specific environment variables now.

ccasin commented 6 years ago

With resepect to the question of whether it was previously "fully fixed" - at the time these changes were made, qemu was not a core part of our system. In fact I just added it to the repos.txt file this week.

I am happy to make this change myself, but I definitely want to maintain the world where no ISP-specific environment variables are required. I understand this one doesn't seem that onerous to the user, but it started with just one variable and grew into a giant mess the last time too.

sbrookes commented 5 years ago

@austinharris @ccasin has this been resolved yet?

ccasin commented 5 years ago

No. let's leave the ticket open for now.