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

Document new pants launcher binary, aka scie-pants. #18056

Merged
merged 7 commits into from Jan 24, 2023

Conversation

benjyw
Copy link
Sponsor Contributor

@benjyw benjyw commented Jan 22, 2023

No description provided.

@benjyw
Copy link
Sponsor Contributor Author

benjyw commented Jan 22, 2023

Rendered:

Screenshot 2023-01-22 at 7 13 04 PM

>
> To upgrade the `pants` launcher binary itself, run
> ```
> SCIE_BOOT=update pants
Copy link
Member

Choose a reason for hiding this comment

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

Is this the first reference to SCIE for a user?

I think there needs to be a link to the scie-pants repo, to remove some ambiguity as to what this means.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

I don't think I agree. The user doesn't need more verbiage or terminology. They are just trying to GSD. We can treat this as a magic incantation.

I can add a "If you're interested in how this works, here is the repo", but I don't want to introduce this as something you're expected to read or know about.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is SCIE an acronym that we even need to expose to the user? Why not an environment variable with "Pants" in the name?

Copy link
Contributor

@tdyas tdyas Jan 22, 2023

Choose a reason for hiding this comment

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

Or pants self update like rustup self update?

Copy link
Member

Choose a reason for hiding this comment

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

"If you're interested in how this works, here is the repo"

I mean, at the very least, something like this - especially since the script is downloading a binary from the scie-pants repo, not this repo.

Is SCIE an acronym that we even need to expose to the user? Why not an environment variable with "Pants" in the name?

Or if something like this happens.

I'm personally very hesitant of one-line, bash-script installers - so to have that, and then have a strange acronym for the update path. As a first time user, that would feel weird without some context.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

@sureshjoshi you may be looking at this from the perspective of someone who wants to poke under the hood (as you indeed do). I think almost all users just want to get things working with the minimal amount of cognitive load. So I'm leery of introducing new concepts, repos, links etc. I'd rather we implemented pantsbuild/scie-pants#32 to get rid of the strange acronym, but in the short term saying "this is a magic incantation" seems OK? Explaining it adds cognitive load, and I'm not sure what is gained in exchange.

If you're hesitant of one-line bash installers, that's a separate issue, because that's 100% what this change is... What would your alternative be?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not a one-line bash person, and that's fine - won't ever be one. Many workarounds in my life (Brew, for reasons detailed in Slack and other).

I'm generally not interested in poking under the hood, believe it or not - it's just when new, unknown terms are involved - I tend to like to see them defined rather than "trust us, this is the way". More of a good OSS citizen, rather than any actual interest in underlying process.

I can't say it much better than John:

The former leaks an implementation detail. A Pants user should not need to be confronted with what scie means. The latter is just plain clunky.

I agree that the linked issue is a better overall solution - and if that's the plan, then whatever comments I have about this page can be deferred to a later date to hopefully not need them.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

If we could get pants into standard brew (and apt) that would be great. But once you have to add a custom keg or whatever, the bash thing seems strictly easier to me...

So we're agreed on implementing pantsbuild/scie-pants#32, and meanwhile I have added a "here's how this is implemented, but you don't need to know this" section.

Copy link
Member

Choose a reason for hiding this comment

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

Could add as a trailing section, the option to install with brew?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

Definitely, now that I know it's a thing. Can you confirm the incantation? Post-rename?

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks a lot @jsirois, @benjyw.

@@ -38,25 +33,19 @@ System-specific notes
>
> If you need to run Pants on Alpine, [let us know](doc:community), so we can prioritize this work. Meanwhile, you can try [building Pants yourself](doc:manual-installation#building-pants-from-sources) on Alpine.

> 🚧 Linux on ARM is not yet supported
> 🚧 Linux on ARM will be supported from Pants 2.16
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Since these are the Pants 2.16 docs, this section could be present tense. Unless you're planning on cherry-picking this change to 2.15...?

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

I was planning on cherrypicking, possibly even to 2.14.x, so we can get this rolling sooner.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Hm. I think that given the questions from the meeting, we might want to have it be the recommended approach for installing 2.16 first, and then only cherry-pick to older versions if that goes well? But I don't feel strongly about it.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

The audience is primarily new users, so why would it matter which version of Pants we target?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The audience is primarily new users, so why would it matter which version of Pants we target?

Because of the volume of users that we get on stable vs unstable versions. Trying this out with non-power users in a low-volume situation (alphas and release candidates for a new release) as opposed to an already stable release might better align with expectations.

>
> The `./pants` script will automatically install and use the Pants version specified in `pants.toml`, so upgrading Pants is as simple as editing `pants_version` in that file.
> We strongly recommend removing the `./pants` script from your repo and using the `pants` binary instead. You can keep a simple `./pants` script that delegates to `pants` to ease the transition. However, if you do need to continue to use the old installation method for some reason, it is described [here](doc:manual-installation). But please [let us know](doc:getting-help) so we can accommodate your use case in the launcher binary.
Copy link
Contributor

Choose a reason for hiding this comment

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

From the perspective of someone advocating for the adoption of pants, "there is a ./pants script and it takes care of bootstrapping everything" is a nice property. Would it ease the transition if there was a ./pants script that either downloaded/cached the binary, or invoked said binary? (I know this a docs PR so that discussion might have already happened elsewhere.)

If I wanted to use such a script internally, I'm not sure from the "strongly recommend" language if that is a terrible idea or not.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

What we're trying to get away from is having that script do all the laborious bootstrapping work it currently does. But you're certainly welcome to have a ./pants script that delegates to pants and, if it's missing, either tells users how to install that, or actually installs it.

We haven't (yet?) created one because:
A) we're not sure we want to recommend that for new users
B) we're not sure what such a script should do. E.g., should it install for you, or just tell you how to install? How would it manage adding ~/bin to your PATH?

Also, and I have not yet added this to this PR, but macOS users can also install via brew, so that's a whole other installation mechanism.

So we're not sure there's a one-size-fits-all ./pants script here, and for now it may make sense for repos to write what makes sense for them.

Copy link
Member

Choose a reason for hiding this comment

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

I second leaving this to end users to come up with, as it'll be a small-ish script if used at all, adapted to each situation. We could provide an example or two in docs for what it could look like for a few common cases, perhaps.

Using our one-step setup script
-------------------------------
```
/bin/bash -c "$(curl -fsSL https://static.pantsbuild.org/setup/pantsup.sh)"
Copy link
Contributor

Choose a reason for hiding this comment

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

For people who don't trust the curl|bash pattern, it would be mildly nice if this url displayed as text in a browser (a-la github raw view) instead triggering a download.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

Hmm, right now the content-type is detected by the server as application/x-sh. Not sure how to change that on our current hosting setup.

Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

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

And that does seem like the appropriate content-type?

@benjyw benjyw added this to the 2.15.x milestone Jan 24, 2023
@benjyw benjyw merged commit ee99812 into pantsbuild:main Jan 24, 2023
@benjyw benjyw deleted the scie_pants_documentation branch January 24, 2023 14:55
benjyw added a commit to benjyw/pants that referenced this pull request Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants