Skip to content

Add tooling utils - #15

Closed
jcbhmr wants to merge 9 commits into
mainfrom
add-tooling-utils
Closed

Add tooling utils#15
jcbhmr wants to merge 9 commits into
mainfrom
add-tooling-utils

Conversation

@jcbhmr

@jcbhmr jcbhmr commented May 1, 2023

Copy link
Copy Markdown
Member

This PR would...

  • Make it so that there's a prebuild script that copies util into each src folder
  • Run prebuild in the github actions prior to publication and testing
  • make a wrapper test.sh script that runs prebuild, then delegates to devcontainer features test
  • add some util scripts for cmake and llvm (see below)

if the proposal to make shared stuff is already here, then this is redundant! yay! 🥳
devcontainers/spec#209

it just needs to be implemented in the cli... until then, we're stuck doing the cp utils src/$ID/utils manually 🤷‍♀️

@jcbhmr jcbhmr added the enhancement New feature or request label May 1, 2023
@jcbhmr jcbhmr self-assigned this May 1, 2023
@eitsupi

eitsupi commented May 1, 2023

Copy link
Copy Markdown
Contributor

Is there any reason why we can't wait for devcontainers/spec#209 to be implemented in the CLI?

@jcbhmr

jcbhmr commented May 1, 2023

Copy link
Copy Markdown
Member Author

Is there any reason why we can't wait for devcontainers/spec#209 to be implemented in the CLI?

no reason whatsoever! if we can wait until then, that's less work for me 👍

@eitsupi

eitsupi commented May 1, 2023

Copy link
Copy Markdown
Contributor

I do not think it is worthwhile to deviate from the standard workflow to implement this.

@jcbhmr jcbhmr mentioned this pull request May 1, 2023
3 tasks
@jcbhmr

jcbhmr commented May 1, 2023

Copy link
Copy Markdown
Member Author

sounds like a plan! 🚀 should i just copy the two script into the src/id/utils folder manually for now then? 🤔

# shellcheck shell=bash

clear_local_apt_index() (
  set -e
  
  rm -rf /var/lib/apt/lists/*
  echo '🟩 Cleared local apt index'
)
# shellcheck shell=bash

ensure_apt_packages() (
  set -e

  export DEBIAN_FRONTEND=noninteractive
  if dpkg -s "$@" &>/dev/null; then
    echo "🟦 $@ is already installed"
  else
    if [[ $(find /var/lib/apt/lists/* | wc -l) == 0 ]]; then
      echo '🟪 Updating local apt index...'
      apt-get update -y
      echo '🟩 Updated local apt index'
    fi
    echo "🟪 Installing $@..."
    apt-get install -y --no-install-recommends "$@"
    echo "🟩 Installed $@"
  fi
)

@jcbhmr jcbhmr closed this May 1, 2023
jcbhmr added a commit that referenced this pull request May 1, 2023
@eitsupi

eitsupi commented May 1, 2023

Copy link
Copy Markdown
Contributor

I think manual copying is best at this time.

I would prefer not to split scripts for functions of that length.
Without splitting, it would be very easy to simply copy the script and run it.

@jcbhmr
jcbhmr deleted the add-tooling-utils branch May 2, 2023 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants