Skip to content

fix(gh-inputs): support maps of size 1#11

Merged
alandefreitas merged 2 commits intoalandefreitas:developfrom
Nerixyz:fix/get-map
Mar 17, 2025
Merged

fix(gh-inputs): support maps of size 1#11
alandefreitas merged 2 commits intoalandefreitas:developfrom
Nerixyz:fix/get-map

Conversation

@Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Mar 15, 2025

I was wondering why subrange-policy: "msvc: one-per-minor" didn't work for me1. It turns out, parseMap will emit the first entry as {key: "msvc", value: "one-per-minor"}.

Footnotes

  1. It doesn't work in this repository either - see line 139 in Generate Test Matrix in https://github.com/alandefreitas/cpp-actions/actions/runs/12718660506/job/35457506938.

@alandefreitas
Copy link
Owner

Oh... I see.

You must also run build.sh and include the changes in the commit.

That's unfortunate but it's how GHA works.

@Nerixyz
Copy link
Contributor Author

Nerixyz commented Mar 17, 2025

Hmm, I had to modify the build.sh script to include the projects in common/. Could be helpful to turn the project into an npm/pnpm workspace. There, you can run one command for all packages and the packages can share dependencies.

build.sh Diff
diff --git a/build.sh b/build.sh
old mode 100644
new mode 100755
index 36d3909..6fde3a9
--- a/build.sh
+++ b/build.sh
@@ -1,20 +1,8 @@
+#!/usr/bin/bash
 # Description: Build all the javascript projects in the repository
 
-projects_with_package=()
-projects_with_action=()
-
-for dir in */; do
-    # Ignore the docs directory
-    if [ "$dir" == "docs/" ]; then
-        continue
-    fi
-
-    if [ -f "$dir/package.json" ]; then
-        projects_with_package+=("$dir")
-    elif [ -f "$dir/action.yml" ]; then
-        projects_with_action+=("$dir")
-    fi
-done
+mapfile -t projects_with_package < <(find . -type f -name "package.json" ! -path "*/node_modules/*" ! -path "*/docs/*" -exec dirname {} \; | sed 's|^\./||')
+mapfile -t projects_with_action < <(find . -type f -name "action.yml" ! -path "*/node_modules/*" ! -path "*/docs/*" -exec dirname {} \; | sed 's|^\./||')
 
 project_to_build=$1

@alandefreitas
Copy link
Owner

alandefreitas commented Mar 17, 2025

GHA doesn't need the build directory for the projects in the common directory because they're not actions and other projects already include them in their package.json.

So commits shouldn't include their build directories. They're already listed in .gitignore.

Could be helpful to turn the project into an npm/pnpm workspace. There, you can run one command for all packages and the packages can share dependencies

Yes. I'd have to study that. :)

@alandefreitas
Copy link
Owner

Could you confirm these failures are unrelated to your changes?

I believe they are because these actions are living things, and new problems with new containers must be fixed occasionally.

@Nerixyz
Copy link
Contributor Author

Nerixyz commented Mar 17, 2025

GHA doesn't need the build directory for the projects in the common directory because they're not actions and other projects already include them in their package.json.

Oh yea, they don't need to be built, but they need to have their (dev-)dependencies installed, because they have a prepare script which needs ncc and runs after they themselves have been installed as a dependency.

Changing prepare to build might help, although, Node still seems to struggle to find the dependencies from packages in common/ (like @actions/core) when running the tests. Not sure why.

Could you confirm these failures are unrelated to your changes?

Tried a blank run in Nerixyz#1 (with just an empty commit). The fmt issue could probably be solved by updating it.

@alandefreitas alandefreitas merged commit 93929e5 into alandefreitas:develop Mar 17, 2025
33 of 35 checks passed
@alandefreitas
Copy link
Owner

Mmm... I never had theses problems but I guess that's more of a problem with the manifest files anyway.

Thanks! :)

@Nerixyz Nerixyz deleted the fix/get-map branch March 17, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants