Skip to content

Add tooling utils #15

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

Closed
wants to merge 9 commits into from
Closed

Add tooling utils #15

wants to merge 9 commits into from

Conversation

jcbhmr
Copy link
Member

@jcbhmr jcbhmr commented May 1, 2023

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
Copy link
Contributor

eitsupi commented May 1, 2023

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

@jcbhmr
Copy link
Member Author

jcbhmr commented May 1, 2023

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
Copy link
Contributor

eitsupi commented May 1, 2023

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
Copy link
Member Author

jcbhmr commented May 1, 2023

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
Copy link
Contributor

eitsupi commented May 1, 2023

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