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

Add helper functions to print to log/print to stderr #331

Open
mafredri opened this issue Oct 24, 2024 · 6 comments
Open

Add helper functions to print to log/print to stderr #331

mafredri opened this issue Oct 24, 2024 · 6 comments
Labels
module-idea A new module idea or suggestion

Comments

@mafredri
Copy link
Member

While reviewing #329 I noticed we didn't print any errors to stderr. I suggest we add helper functions for this:

# lib
error() {
    echo "ERROR: $@" >&2
    exit 1 # (optional)
}

log() {
    echo "$@"
}

Then use them where appropriate:

log "Installing package..."
install || error "Failed to install package"

// cc @matifali

@coder-labeler coder-labeler bot added the module-idea A new module idea or suggestion label Oct 24, 2024
@matifali
Copy link
Member

matifali commented Oct 24, 2024

Great idea. To make these helper functions available across modules. We may have to make used a common module that gets nested in each module.

A similar proposal is #312

@mafredri
Copy link
Member Author

mafredri commented Oct 24, 2024

We may have to make used a common module that gets nested in each module.

I’ve been thinking about this on and off myself. A common module is definitely one option, but I’ve also been entertaining the idea of using code-gen to bundle the common code into all modules. Not sure what would be the best approach.

EDIT: I say code-gen, but it could just as well be done as built-time transformation.

@matifali
Copy link
Member

Yes. Generated code is fine too. The idea is to reduce fragmentation and optimize the UX.

@mafredri
Copy link
Member Author

mafredri commented Oct 25, 2024

@matifali I've been thinking about how to solve this from a couple of perspectives:

  1. How do we include common code (lib.sh) for all modules?
  2. How can we make sure our code can be validated by shellcheck?
  3. How do we ensure TF variables are available in code
  4. How do we ensure we don't make errors where we write ${VAR} and get non-existent TF var vs intended shell var

Essentially, this is what I've come up with:

resource "coder_script" "vscode-web" {
  script = templatefile("${path.module}/script.tftpl", {
    LOADER: "/usr/bin/env bash",
    LIB: file("${path.module}/lib.sh"),
    RUN: file("${path.module}/run.sh"),
    VARS: templatefile("${path.module}/vars.tftpl", {
      PORT : var.port,
      LOG_PATH : var.log_path,
      INSTALL_PREFIX : var.install_prefix,
      EXTENSIONS : join(",", var.extensions),
      TELEMETRY_LEVEL : var.telemetry_level,
      // This is necessary otherwise the quotes are stripped!
      SETTINGS : replace(jsonencode(var.settings), "\"", "\\\""),
      OFFLINE : var.offline,
      USE_CACHED : var.use_cached,
      EXTENSIONS_DIR : var.extensions_dir,
      FOLDER : var.folder,
      AUTO_INSTALL_EXTENSIONS : var.auto_install_extensions,
    }),
  })
}

Where:

  • script.tftpl is generated/based on a template
  • lib.sh is copied to all modules (from e.g. .scripts/lib.sh)
  • run.sh is a pure editable shell scripts, no TF variables, shellcheck and shfmt works
  • vars.tftpl is autogenerated from main.tf

script.tftpl:

#!${LOADER}

# lib.sh
touch "$CODER_SCRIPT_DATA_DIR/lib.sh"
${LIB}

# vars.tftpl
touch "$CODER_SCRIPT_DATA_DIR/vars.tftpl"
${VARS}

# run.sh
${RUN}

(The touches are present here to avoid errors when executing run.sh content and sourcing the files. Alternatively the source lines could be stripped out but puts logic in main.tf.)

lib.sh:

#!/bin/sh

log() { echo "$@" }

run.sh:

#!/usr/bin/env bash

# shellcheck source=vscode-web/lib.sh
. "${CODER_SCRIPT_DATA_DIR}/lib.sh"
# shellcheck source=vscode-web/vars.tftpl
. "${CODER_SCRIPT_DATA_DIR}/vars.tftpl"

run_vscode_web() {
  log running... # log from lib.sh
  # ...

vars.tftpl:

#!/usr/bin/env sh
# Code generated by [insert name]. DO NOT EDIT.

# shellcheck disable=SC2269
PORT="${PORT}" # type: number, default: 13338, description: The port to run VS Code Web on.
LOG_PATH="${LOG_PATH}" # ...
INSTALL_PREFIX="${INSTALL_PREFIX}"
EXTENSIONS="${EXTENSIONS}"
TELEMETRY_LEVEL="${TELEMETRY_LEVEL}"
SETTINGS="${SETTINGS}"
OFFLINE="${OFFLINE}"
USE_CACHED="${USE_CACHED}"
EXTENSIONS_DIR="${EXTENSIONS_DIR}"
FOLDER="${FOLDER}"
AUTO_INSTALL_EXTENSIONS="${AUTO_INSTALL_EXTENSIONS}"

End result (coder script):

#!/usr/bin/env bash

# lib.sh
touch "$CODER_SCRIPT_DATA_DIR/lib.sh"
#!/bin/sh

log() { echo "$@" }

# vars.tftpl
touch "$CODER_SCRIPT_DATA_DIR/vars.tftpl"
#!/usr/bin/env sh
# Code generated by [insert name]. DO NOT EDIT.

# shellcheck disable=SC2269
PORT="${PORT}" # type: number, default: 13338, description: The port to run VS Code Web on.
LOG_PATH="${LOG_PATH}" # ...
INSTALL_PREFIX="${INSTALL_PREFIX}"
EXTENSIONS="${EXTENSIONS}"
TELEMETRY_LEVEL="${TELEMETRY_LEVEL}"
SETTINGS="${SETTINGS}"
OFFLINE="${OFFLINE}"
USE_CACHED="${USE_CACHED}"
EXTENSIONS_DIR="${EXTENSIONS_DIR}"
FOLDER="${FOLDER}"
AUTO_INSTALL_EXTENSIONS="${AUTO_INSTALL_EXTENSIONS}"

# run.sh
#!/usr/bin/env bash

# shellcheck source=vscode-web/lib.sh
. "${CODER_SCRIPT_DATA_DIR}/lib.sh"
# shellcheck source=vscode-web/vars.tftpl
. "${CODER_SCRIPT_DATA_DIR}/vars.tftpl"

run_vscode_web() {
  log running... # log from lib.sh
  # ...

Not sure if this will make it hard to understand the project structure and how/where to contribute. I have some other ideas too, but that requires introducing a pre-filter for shellcheck and supporting shfmt would be hard. Also you'd still have to be careful with $${VAR} vs ${VAR} but we could introduce a new linter for that and enforce that all tf is at the top of the file, for instance.

Also, it'd be possible to combine script.tftpl and vars.tftpl if that makes things easier, but at the same time it'd be a bit of inception (run.sh imports script.tftpl and script.tftpl embeds run.sh 😂)

@matifali
Copy link
Member

This is an exciting approach and will greatly improve quality and maintenance. and if we can automate it with a script like bun generate, I think it will help make the adoption easy for adoption. Let it stay here fir a few weeks to get more feedback from the community.

@matifali
Copy link
Member

As we move over to the refactored registry and start versioning each module individually, I think this would be a good addition. Let me know if you want to work on this in the next cycle. Happy to sync on this to discuss it further.

What is important for me is to,

  1. Ensure we reuse as much of the common code as possible.
  2. Easy to contribute and maintain for new contributors. (A nice CONTRIBUTIN.md can help).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module-idea A new module idea or suggestion
Projects
None yet
Development

No branches or pull requests

2 participants