-
Notifications
You must be signed in to change notification settings - Fork 17
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
base: main
Are you sure you want to change the base?
Clarify recommended install steps #68
Conversation
Signed-off-by: Andy Fingerhut <[email protected]>
I know that 75% of people will not read it, but at least we can point them to it afterwards. Signed-off-by: Andy Fingerhut <[email protected]>
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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"?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 thep4studio 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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]>
…users Signed-off-by: Andy Fingerhut <[email protected]>
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]>
No description provided.