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

Clarify recommended install steps #68

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

jafingerhut
Copy link
Contributor

No description provided.

README.md Outdated
@@ -49,6 +49,17 @@ Some things not included, that one must get from Intel:

# Setup

If you attempt to install this software on a system that already has other
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider that a bug in our installation scripts if that happens. Ideally, they are hermetic. Fwiw, I have a "dirty" local machine because I also have a normal P4C environment, but it works well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would only keep the second path.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing where we run into trouble are the "OS" dependencies listed here:
https://github.com/p4lang/open-p4studio/blob/main/p4studio/dependencies/dependencies.yaml#L5

These are global and system specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would only keep the second path.

You mean, the p4studio interactive install step currently in the top level README, and delete the p4studio profile apply testing.yaml one?

If so, should we recommend a particular set of options as "here are the set of enabled options most well tested, so use them if you do not have a preference otherwise"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would consider that a bug in our installation scripts if that happens. Ideally, they are hermetic. Fwiw, I have a "dirty" local machine because I also have a normal P4C environment, but it works well.

Perhaps, but who wants to debug it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would only keep the second path.

You mean, the p4studio interactive install step currently in the top level README, and delete the p4studio profile apply testing.yaml one?

If so, should we recommend a particular set of options as "here are the set of enabled options most well tested, so use them if you do not have a preference otherwise"?

Ah typo, meant to say paragraph.

Perhaps, but who wants to debug it?

It's really difficult to keep a machine clean. Ideally, our installation script should be consistent enough that you do not have to worry about that as a user. That also makes it much easier to change dependencies or upgrade them. For example, with P4C we used to have a lot of problems with Boost and Protobuf. These problems are largely resolved because we now control these dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am now recalling from years back that Vladimir Gurevich has also said that with the proprietary Intel P4Studio, he often installed more than one version side-by-side in different directories, and it all worked, as long as he used the appropriate environment variable settings. So sorry for firing off this PR without thinking it through carefully enough.

I will rework this PR, or discard it and create another, to emphasize what seems to be the root cause of the recent issue someone was asking about, which is to emphasize the criticality of creating and using the setup-open-p4studio.bash script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we need to emphasize that you either need to set up the environment variables, or use a wrapper that does it for you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated this PR, with an attempt at making it as explicit as I know how, how important this setup script is.

Nothing is foolproof, of course. I can lead a horse to water, but I can't make it drink.

Signed-off-by: Andy Fingerhut <[email protected]>
by mentioning the options.  They already chose the batch install.

Signed-off-by: Andy Fingerhut <[email protected]>
The fewer number of words, the more likely someone will read
and understand the ones that are there.

Signed-off-by: Andy Fingerhut <[email protected]>
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