search

automotiveMastermind / prompt

A spectacular prompt for *nix distributions.
MIT License
9 stars 6 forks source link

feat: add support for derived distributions and gnome terminal themes #82

Closed dmccaffery closed 3 years ago

dmccaffery commented 3 years ago
dmccaffery commented 3 years ago

derived install tested on Pop! OS: image

dmccaffery commented 3 years ago

Yeah; something changed in how WSL is implemented in GitHub actions; gosu no longer persists through sudo, so homebrew installer is chowning the path as root:build with u=rw,go=r -- and the clone of the homebrew repo fails.

I'll be looking into it when I have a chance.

dmccaffery commented 3 years ago

@dmccaffery looks like there are still some CI issues :/

I cloned and installed your branch locally and the issues I was having with zsh completions are resolved. So far I haven't run into any other issues which is a positive.

Cloning and installing this via install.sh did make me wonder if there's value in having a flag that would allow you to pass a GitHub url, so you could test installing via the bootstrap script? Might also help with CI since we were calling install directly there too, right?

re: passing in a git-url -- I'm not sure I understand the ask -- do you mean to run the bootstrap script against an arbitrary url, including forks? -- if so, that is incredibly dangerous -- its a huge security risk as one could redirect the bootstrap script to do whatever it wants, even from ci/cd on a fork -- the bootstrap script also calls install directly; all that it actually does is to extract a tarball of the repo into a temp directory and then invoke the install script. The real work achieved by the bootstrap script is to download the tarball... doing that against an arbitrary repo isn't helpful, I don't think.

dmccaffery commented 3 years ago

@patrickserrano : this is all that bootstrapping does:

https://github.com/automotiveMastermind/prompt/blob/d1eaccb93a7004ff382e94f2ca72b82749adb3b1/bootstrap.sh#L87-L90

The rest is just a comparison of the git-ref against the current upstream.

dmccaffery commented 3 years ago

@patrickserrano / @sjk07 : should be ready for review / approval -- all builds should now pass. I had to do some gymnastics with the end-to-end build to get wsl to work again.

patrickserrano commented 3 years ago

@dmccaffery looks like there are still some CI issues :/ I cloned and installed your branch locally and the issues I was having with zsh completions are resolved. So far I haven't run into any other issues which is a positive. Cloning and installing this via install.sh did make me wonder if there's value in having a flag that would allow you to pass a GitHub url, so you could test installing via the bootstrap script? Might also help with CI since we were calling install directly there too, right?

re: passing in a git-url -- I'm not sure I understand the ask -- do you mean to run the bootstrap script against an arbitrary url, including forks? -- if so, that is incredibly dangerous -- its a huge security risk as one could redirect the bootstrap script to do whatever it wants, even from ci/cd on a fork -- the bootstrap script also calls install directly; all that it actually does is to extract a tarball of the repo into a temp directory and then invoke the install script. The real work achieved by the bootstrap script is to download the tarball... doing that against an arbitrary repo isn't helpful, I don't think.

I hadn't really considered the security implications of such a change sine I was thinking of ease of manual testing larger PRs. But it makes sense why we wouldn't want to do that.

dmccaffery commented 3 years ago

:tada: This PR is included in version 8.3.0-next.3 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: