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

feat/add fishtape option to fish feature #25

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Contribute

## Install

```console
yarn install
Copy link
Member

@andreiborisov andreiborisov Nov 10, 2022

Choose a reason for hiding this comment

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

We already use pnpm as it's faster compared to yarn.

Copy link
Member

@andreiborisov andreiborisov Nov 10, 2022

Choose a reason for hiding this comment

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

Contributing guidelines is a huge topic for Meaningful, so they're developed separately to be used organization-wide: https://linear.app/meaningful/issue/GRA-8/contributing-guidelines

The goal is to be as modular and reusable as we possibly can.

```

## Test

**requirements:** [`just`][just]

Pass a `feature` and a `base image`, _e.g._:

```fish
just test fish mcr.microsoft.com/devcontainers/base:debian
Copy link
Member

Choose a reason for hiding this comment

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

There is also https://github.com/go-task/task which looks good. Before committing to something, we should compare those (sorry for being pedantic, I'd like those decisions to be organization-wide so it requires a certain level of scrupulousness).

```

[just]: https://github.com/casey/just
10 changes: 10 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
test feature image:
devcontainer features test \
--skip-scenarios \
--features {{feature}} \
--base-image {{image}} .

test-with-scenarios feature image:
devcontainer features test \
--features {{feature}} \
--base-image {{image}} .
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "devcontainer-features",
"devDependencies": {
"@devcontainers/cli": "^0.23.2",
"gitmoji-cli": "^7.0.2"
}
}
12 changes: 7 additions & 5 deletions src/fish/README.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@

# fish (fish)

Installs fish shell and Fisher plugin manager (optionally)
Installs fish shell and [Fisher][fisher] plugin manager and optionally [Fishtape][fishtape] test runner.

## Example Usage

```json
"features": {
"ghcr.io/meaningful-ooo/devcontainer-features/fish:1": {
"version": "latest"
"fishtape": true
}
}
```
Expand All @@ -17,10 +17,12 @@ Installs fish shell and Fisher plugin manager (optionally)

| Options Id | Description | Type | Default Value |
|-----|-----|-----|-----|
| fisher | Install Fisher plugin manager | boolean | true |


| `fisher` | Install [Fisher][fisher] plugin manager | boolean | `true` |
| `fishtape` | Install [Fishtape][fishtape], 100% pure-Fish test runner | boolean | `false` |

---

_Note: This file was auto-generated from the [devcontainer-feature.json](https://github.com/meaningful-ooo/devcontainer-features/blob/main/src/fish/devcontainer-feature.json). Add additional notes to a `NOTES.md`._

[fisher]: https://github.com/jorgebucaran/fisher
[fishtape]: https://github.com/jorgebucaran/fishtape
Comment on lines +26 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not edit README.md directly as it is automatically generated from devcontainer-feature.json and NOTES.md.

5 changes: 5 additions & 0 deletions src/fish/devcontainer-feature.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@
"type": "boolean",
"default": true,
"description": "Install Fisher plugin manager"
},
"fisher_plugins": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be camelCase.

Suggested change
"fisher_plugins": {
"fisherPlugins": {

"type": "string",
"default": "",
"description": "Install [Fish's plugins](https://github.com/jorgebucaran/awsm.fish) using `fisher`"
}
},
"customizations": {
Expand Down
19 changes: 19 additions & 0 deletions src/fish/install.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env bash

FISHER=${FISHER:-"true"}
FISHER_PLUGINS=${FISHER_PLUGINS:-""}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
FISHER_PLUGINS=${FISHER_PLUGINS:-""}
FISHERPLUGINS=${FISHERPLUGINS:-""}

Copy link
Member

@andreiborisov andreiborisov Nov 10, 2022

Choose a reason for hiding this comment

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

I propose to go with the same approach maintainers currently use in https://github.com/devcontainers/features: devcontainers/spec#57 (comment)

I think it's a little bit more readable in options and they can effectively migrate us later when the proper arrays will become supported:

BASH_ARRAY=( ${COMMA_SEPARATED_OPTION//,/ } )

Vote with 👍 or 👎, please.

USERNAME=${USERNAME:-"automatic"}

source /etc/os-release
Expand Down Expand Up @@ -118,6 +119,24 @@ if [ "${FISHER}" = "true" ]; then
fish -c "fisher -v"
fi

# Install Fisher plugins
if [ -n "${FISHER_PLUGINS}" ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

fisher should be installed.

Suggested change
if [ -n "${FISHER_PLUGINS}" ]; then
if [ "${FISHER}" = "true" ] && [ -n "${FISHERPLUGINS}" ]; then

echo "Installing Fisher plugins…"
for plugin in "${FISHER_PLUGINS[@]}"; do
printf "\tInstalling plugin: ${plugin}"
fish -c "fisher install ${FISHER_PLUGINS}"
done

if [ "${USERNAME}" != "root" ]; then
for plugin in "${FISHER_PLUGINS[@]}"; do
printf "\tInstalling plugin: ${plugin}"
su $USERNAME -c "fish -c 'fisher install ${FISHER_PLUGINS}'"
done
fi
fish -c 'fisher list; cat $__fish_config_dir/fish_plugins'
fi


# Clean up
cleanup

Expand Down
27 changes: 27 additions & 0 deletions test/fish/alpine_with_fisher_multiple_plugins.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#!/usr/bin/env bash

set -e

NON_ROOT_USER=""
POSSIBLE_USERS=("vscode" "node" "codespace" "$(awk -v val=1000 -F ":" '$3==val{print $1}' /etc/passwd)")
for CURRENT_USER in "${POSSIBLE_USERS[@]}"; do
if id -u ${CURRENT_USER} >/dev/null 2>&1; then
NON_ROOT_USER=${CURRENT_USER}
break
fi
done
if [ "${NON_ROOT_USER}" = "" ]; then
NON_ROOT_USER=root
fi

# Optional: Import test library bundled with the devcontainer CLI
source dev-container-features-test-lib

check "fish" fish -v
echo "Testing with user: ${NON_ROOT_USER}"
check "fisher" su "${NON_ROOT_USER}" -c 'fish -c "fisher -v"'
check "fishtape has been installed" su "${NON_ROOT_USER}" -c 'fish -c "type -q fishtape"'
check "pure has been installed" su "${NON_ROOT_USER}" -c 'fish -c "set --query pure_version"'

# Report result
reportResults
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ source dev-container-features-test-lib
check "fish" fish -v
echo "Testing with user: ${NON_ROOT_USER}"
check "fisher" su "${NON_ROOT_USER}" -c 'fish -c "fisher -v"'
check "fishtape has been installed" su "${NON_ROOT_USER}" -c 'fish -c "type -q fishtape"'

# Report result
reportResults
15 changes: 13 additions & 2 deletions test/fish/scenarios.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,19 @@
{
"alpine": {
"alpine_with_fisher_single_plugin": {
"image": "mcr.microsoft.com/devcontainers/base:alpine",
"features": {
"fish": {}
"fish": {
"fisher_plugins": "jorgebucaran/fishtape"
}
}
},
"alpine_with_fisher_multiple_plugins": {
"image": "mcr.microsoft.com/devcontainers/base:alpine",
"features": {
"fish": {
"fisher_plugins": "jorgebucaran/fishtape pure-fish/pure"
}
}
}
}

2 changes: 2 additions & 0 deletions test/fish/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ case "${ID}" in
esac
echo "Testing with user: ${NON_ROOT_USER}"
check "fisher" su "${NON_ROOT_USER}" -c 'fish -c "fisher -v"'
ONLY_FISHER_IS_INSTALL=1
check "no fisher plugins are installed by default" test "$(su "${NON_ROOT_USER}" -c 'fish -c "fisher list" | wc -l')" -eq $ONLY_FISHER_IS_INSTALL

# Report result
reportResults
Loading