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

Upgrade pymdown-extensions and markdown #3053

Merged
merged 2 commits into from
Nov 4, 2023

Conversation

BryanQuigley
Copy link
Contributor

Also merges where python dependencies are specified to requirements.txt.

Fixes #3041

Proposed Changes

  1. Add restrict_base_path: False to mkdocs - this unblocks upgrading pymdown
  2. Consolidate where packages are specified to just use requirements.txt

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

@BryanQuigley
Copy link
Contributor Author

I've only tested the local dev workflow, but I believe the others should work fine.

.github/workflows/build-deploy-docs.yml Show resolved Hide resolved
entrypoint.sh Outdated
@@ -35,7 +35,7 @@ if [ "${UPGRADE_LINTERS_VERSION}" == "true" ]; then
# Run only get_linter_help test methods
pytest -v --durations=0 -k _get_linter_help megalinter/
# Reinstall mkdocs-material because of broken dependency
pip3 install --upgrade "markdown==3.3.7" mike mkdocs-material "pymdown-extensions==9.11" "mkdocs-glightbox==0.3.2" mdx_truly_sane_lists jsonschema json-schema-for-humans giturlparse webpreview "github-dependents-info==0.10.0"
pip3 install --upgrade -r .config/python/dev/requirements.txt
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This however might not work, since I don't think that that file is copied inside the released docker images. The entrypoint.sh script is what is ran when running the docker images. Should we either include the dependencies file, or find a way to keep the deps needed (and not installed by default, like node's dev dependencies), or keep it manually in sync?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted (with changed versions) and added note to keep in sync.

.github/CONTRIBUTING.md Outdated Show resolved Hide resolved
@BryanQuigley BryanQuigley temporarily deployed to dev October 30, 2023 21:45 — with GitHub Actions Inactive
@BryanQuigley BryanQuigley temporarily deployed to dev October 30, 2023 21:45 — with GitHub Actions Inactive
@echoix
Copy link
Collaborator

echoix commented Oct 30, 2023

Did you have the chance to run your built image with the env var UPGRADE_LINTERS_VERSION set to true, in order to see if the following section works?

megalinter/entrypoint.sh

Lines 29 to 41 in e2d2f6e

# Called by Auto-update CI job
if [ "${UPGRADE_LINTERS_VERSION}" == "true" ]; then
echo "[MegaLinter init] UPGRADING LINTER VERSION"
pip install pytest-cov pytest-timeout
# Run only get_linter_version test methods
pytest -v --durations=0 -k _get_linter_version megalinter/
# Run only get_linter_help test methods
pytest -v --durations=0 -k _get_linter_help megalinter/
# Reinstall mkdocs-material because of broken dependency
pip3 install --upgrade "markdown==3.3.7" mike mkdocs-material "pymdown-extensions==9.11" "mkdocs-glightbox==0.3.2" mdx_truly_sane_lists jsonschema json-schema-for-humans giturlparse webpreview "github-dependents-info==0.10.0"
cd /tmp/lint || exit 1
chmod +x build.sh
GITHUB_TOKEN="${GITHUB_TOKEN}" bash build.sh --doc --dependents --stats

It is the equivalent of

#####################################
# Collect linters versions & help #
#####################################
- name: Collect latest versions and help
id: compute_versions
shell: bash
run: docker run -e UPGRADE_LINTERS_VERSION=true -e GITHUB_SHA=${{ github.sha }} -e GITHUB_TOKEN=${GITHUB_TOKEN} -e GITHUB_OUTPUT="${GITHUB_OUTPUT}" -e MEGALINTER_VOLUME_ROOT="${GITHUB_WORKSPACE}" -v "/var/run/docker.sock:/var/run/docker.sock:rw" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v ${GITHUB_WORKSPACE}:/tmp/lint oxsecurity/megalinter:auto_update_${{ github.sha }}
timeout-minutes: 60

@BryanQuigley
Copy link
Contributor Author

Wasn't able to test that path. Should I just revert that part?

@BryanQuigley BryanQuigley temporarily deployed to dev October 31, 2023 22:27 — with GitHub Actions Inactive
@BryanQuigley BryanQuigley temporarily deployed to dev October 31, 2023 22:27 — with GitHub Actions Inactive
@BryanQuigley
Copy link
Contributor Author

I don't believe the GitPod issue was related to this PR.

Also merges where python dependencies are specified to
requirements.txt.

Except for entrypoint.sh which needs to be kepy in sync,
added note in requirement.txt for that.

Fixes: oxsecurity#3041
@echoix
Copy link
Collaborator

echoix commented Nov 3, 2023

Nope, it's fixed now. There was the download server for node that was down a bit

@echoix
Copy link
Collaborator

echoix commented Nov 3, 2023

Lychee failure : wbond/packagecontrol.io#164

@echoix echoix enabled auto-merge (squash) November 4, 2023 00:10
@echoix echoix merged commit 919046c into oxsecurity:main Nov 4, 2023
7 checks passed
@echoix
Copy link
Collaborator

echoix commented Nov 4, 2023

Ooos, seems like Mike was removed and not added to the requirements file. I thought I doubled checked that all the equivalent dependencies were there

@echoix echoix mentioned this pull request Nov 4, 2023
4 tasks
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.

pymdown-extensions version update / consistance in pip package installation
2 participants