diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 75d59b5b0..d0a5bf45c 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -57,11 +57,24 @@ jobs: - name: Test a full build of the app run: npm run build - - name: Test integrity of gitignore.patch + - name: Test integrity of patch_gitignore.sh run: | - # Check that the /scripts/gitignore patch is still valid (it will be needed for the gh-pages implementation used for unbundled testing) - echo "DEV: If the gitignore patch fails, follow instructions in ADDING_DEPENDENCIES_NODE_MODULES.md to correct it." - git apply ./scripts/gitignore.patch + # Check that the /scripts/patch_gitignore script valid (it will be needed for the gh-pages implementation used for unbundled testing) + echo "DEV: If this patch_gitignore script fails, follow instructions in ADDING_DEPENDENCIES_NODE_MODULES.md to correct it." + chmod +x ./scripts/patch_gitignore.sh + ./scripts/patch_gitignore.sh + # Check that .gitignore does NOT contain /dist/ + failed=0 + if grep -q "^/dist/" .gitignore; then + echo "/dist/ found in .gitignore, so patch failed." + failed=1 + fi + # Check that .gitignore DOES contain !/node_modules/ + if ! grep -q "^\!/node_modules/" .gitignore; then + echo "!/node_modules/ not found in .gitignore, so patch failed." + failed=1 + fi + exit $failed - name: End-to-end tests on Chrome (Linux) env: diff --git a/.github/workflows/publish-extension.yaml b/.github/workflows/publish-extension.yaml index 55aa9bed4..19eaf5791 100644 --- a/.github/workflows/publish-extension.yaml +++ b/.github/workflows/publish-extension.yaml @@ -94,8 +94,9 @@ jobs: git config user.email "<>" # Delete gitignore entry for the dist folder sed -i "/^\/dist\/$/d" .gitignore - # Apply the patch to gitignore to allow some dependencies to be included - git apply ./scripts/gitignore.patch + # Patch gitignore so that files needed for the distribution are exposed + chmod +x ./scripts/patch_gitignore.sh + ./scripts/patch_gitignore.sh if [ ! -z "$(git status --porcelain)" ]; then git add . git commit -m "Set GitHub Pages release version" diff --git a/.gitignore b/.gitignore index 7db72b2ca..0b5afc59f 100644 --- a/.gitignore +++ b/.gitignore @@ -43,17 +43,18 @@ build/config/buildinfo.properties www-built www-ghdeploy -/node_modules/ /tmp/ /scripts/kiwix-*.pem /scripts/set_secret_environment_variables.sh /build/ -/dist/ /scripts/travisci_builder_id_key /scripts/ssh_key /scripts/secret_files.tar.gz /reports/ +/dist/ +/node_modules*/ + #Visual Studio /.vs/ /.vscode/ diff --git a/ADDING_DEPENDENCIES_NODE_MODULES.md b/ADDING_DEPENDENCIES_NODE_MODULES.md index 2e4a95083..e288c756e 100644 --- a/ADDING_DEPENDENCIES_NODE_MODULES.md +++ b/ADDING_DEPENDENCIES_NODE_MODULES.md @@ -10,34 +10,29 @@ This syntax is sufficient for the bundler, rollup.js, to pick up the dependency The problem is that the `node_modules` folder is usually ignored so that developers do not submit dependencies to the Repository: instead, each developer should install dependencies using NPM. But unless we make an exception for dependencies needed in the unbundled version, they won't be available when running it in the browser, so we have to ensure they are added **only** to the version published on the dev server. -## Update the `gitignore.patch` (NOT `.gitignore`) +## Update the `patch_gitignore.sh` (NOT `.gitignore`) The app is published to the development server from the `gh-pages` branch. To see the process, look at the workflow `publish-extension.yaml`. It installs dependencies and builds the app. In the workflow, you will see the following lines: ``` -# Apply the patch to gitignore to allow some dependencies to be included -git apply ./scripts/gitignore.patch +# Patch gitignore so that files needed for the distribution are exposed +chmod +x ./scripts/patch_gitignore.sh +./scripts/patch_gitignore.sh ``` +As this implies, you will need to add your new dependency to the `patch_gitignore.sh` script so that `.gitignore` on the `gh-pages` branch is altered, and the dependency is included in the files published to the server. N.B., you must NOT directly commit the changes to `.gitignore` itself, because we do not want multiple versions of dependencies committed to the Repo on every branch and kept forever. The only branch that should have these dependencies committed is `gh-pages`, and that branch is force-pushed each time that the app is published, so only the latest versions of dependencies are committed and kept. -As this implies, you will need to add your new dependency to the `gitignore.patch` so that `.gitignore` on the `gh-pages` branch is altered, and the dependency is included in the files published to the server. N.B., you must NOT directly commit the changes to `.gitignore` itself, because we do not want multiple versions of dependencies committed to the Repo on every branch and kept forever. The only branch that should have these dependencies committed is `gh-pages`, and that branch is force-pushed each time that the app is published, so only the latest versions of dependencies are committed and kept. +To update the patch, you will need to follow this procedure: -To update the patch, you will need the help of the git software to generate the patch, like this: +1. Edit `patch_gitignore.sh` following the example below: -1. On your PR branch make sure your working tree is clean (that you have committed other irrelevant changes -- it's not necessary to have pushed the changes as well); -2. Then run the exising patch with `git apply ./scripts/gitignore.patch`. Do NOT commit the changes to `.gitignore` that this will produce, but do save those changes; -3. Now add your your dependency directly into `.gitignore`, putting it in alphabetical order into the existing block of changes, e.g.: ``` - !/node_modules/i18next - /node_modules/i18next/* - !/node_modules/i18next/dist - /node_modules/i18next/dist/* - !/node_modules/i18next/dist/es - !/node_modules/i18next/dist/es/* + !/node_modules/i18next\ + /node_modules/i18next/*\ + !/node_modules/i18next/dist\ + /node_modules/i18next/dist/*\ + !/node_modules/i18next/dist/es\ + !/node_modules/i18next/dist/es/*\ ``` - This unwieldy syntax is necessary to exclude all the contents of each folder other than the file(s) you wish to include. Note that the `!` at the start of some lines means "do not ignore this folder" (or file). A line without `!` means that the files listed or globbed will all be ignored; -4. Save the file (but don't commit it); -5. Run something like `git diff > mypatch.patch`; -6. Take the code in `mypatch.patch` and overwrite the code (but not the comment) in the current `./scripts/gitignore.patch`; -7. Save the chnages to `gitignore.patch`, and discard the changes to `.gitignore` and `mypatch.patch` (when you discard `.gitignore`, the unignored `node_modules` dependencies should no longer appear as unsstaged changes in your PR); -8. Test your patch works with `git apply ./scripts/gitignore.patch`; -9. If it works, discard the changed `.gitignore` again, and you can finally push (just) the edited `gitignore.patch` to your PR. + This unwieldy syntax is necessary to exclude all the contents of each folder other than the file(s) you wish to include. Note that the `!` at the start of some lines means "do not ignore this folder" (or file). A line without `!` means that the files listed or globbed will all be ignored. +2. Test your patch works by running the script manually (you may need to do `chmod +x` first). You should see the relevant file(s) now appearing as a dirty change in git. DO NOT COMMIT THIS CHANGE! +3. If it works, discard the changed `.gitignore` again, and you can finally push (just) the edited `patch_gitignoe.sh` script to your PR. diff --git a/scripts/gitignore.patch b/scripts/gitignore.patch deleted file mode 100644 index ab7de0199..000000000 --- a/scripts/gitignore.patch +++ /dev/null @@ -1,40 +0,0 @@ -# DEV: This patch file is used in the publish-extension.yaml workflow to allow some dependenicies in node_modules to be committed to the repo -# in the gh-pages branch. This is needed because the gh-pages branch is used to publish an implementation of the app. You may need to maintain -# this list if more dependencies are added to the extension via npm. -# -# >>>>>>>>>>>>>>>>>>>>>>>>>> Add any edied patch below this line <<<<<<<<<<<<<<<<<< -diff --git a/.gitignore b/.gitignore -index 69d180f..45218ed 100644 ---- a/.gitignore -+++ b/.gitignore -@@ -43,7 +43,29 @@ build/config/buildinfo.properties - - www-built - www-ghdeploy --/node_modules/ -+/node_modules/* -+!/node_modules/@fortawesome -+/node_modules/@fortawesome/* -+!/node_modules/@fortawesome/fontawesome-free -+/node_modules/@fortawesome/fontawesome-free/* -+!/node_modules/@fortawesome/fontawesome-free/js -+/node_modules/@fortawesome/fontawesome-free/js/* -+!/node_modules/@fortawesome/fontawesome-free/js/all.js -+!/node_modules/bootstrap -+/node_modules/bootstrap/* -+!/node_modules/bootstrap/dist -+/node_modules/bootstrap/dist/* -+!/node_modules/bootstrap/dist/css -+/node_modules/bootstrap/dist/css/* -+!/node_modules/bootstrap/dist/css/bootstrap.min.* -+!/node_modules/bootstrap/dist/js -+/node_modules/bootstrap/dist/js/* -+!/node_modules/bootstrap/dist/js/bootstrap.bundle.min.* -+!/node_modules/jquery -+/node_modules/jquery/* -+!/node_modules/jquery/dist -+/node_modules/jquery/dist/* -+!/node_modules/jquery/dist/jquery.slim.min.* - /tmp/ - /scripts/kiwix-*.pem - /scripts/set_secret_environment_variables.sh diff --git a/scripts/patch_gitignore.sh b/scripts/patch_gitignore.sh new file mode 100644 index 000000000..bc74c4e70 --- /dev/null +++ b/scripts/patch_gitignore.sh @@ -0,0 +1,30 @@ +#!/bin/bash + +# Delete gitignore entry for the dist folder +sed -i "/^\/dist\/$/d" .gitignore + +# Replace the /node_modules*/ entry in gitignore with the following +sed -i 's|/node_modules\*/|/node_modules/*\ +!/node_modules/@fortawesome\ +/node_modules/@fortawesome/*\ +!/node_modules/@fortawesome/fontawesome-free\ +/node_modules/@fortawesome/fontawesome-free/*\ +!/node_modules/@fortawesome/fontawesome-free/js\ +/node_modules/@fortawesome/fontawesome-free/js/*\ +!/node_modules/@fortawesome/fontawesome-free/js/all.js\ +!/node_modules/bootstrap\ +/node_modules/bootstrap/*\ +!/node_modules/bootstrap/dist\ +/node_modules/bootstrap/dist/*\ +!/node_modules/bootstrap/dist/css\ +/node_modules/bootstrap/dist/css/*\ +!/node_modules/bootstrap/dist/css/bootstrap.min.*\ +!/node_modules/bootstrap/dist/js\ +/node_modules/bootstrap/dist/js/*\ +!/node_modules/bootstrap/dist/js/bootstrap.bundle.min.*\ +!/node_modules/jquery\ +/node_modules/jquery/*\ +!/node_modules/jquery/dist\ +/node_modules/jquery/dist/*\ +!/node_modules/jquery/dist/jquery.slim.min.*\ +|' .gitignore