Skip to content

Commit

Permalink
pre-commit hook migration
Browse files Browse the repository at this point in the history
- Drop the lxml requirement.
- Add new 'fully-loaded' package extra, for all-the-things
- Add warning when using the old ksconf-xml-format hook (from the main repo)
- Prep for v0.12.0 release
  • Loading branch information
lowell80 committed Sep 27, 2023
1 parent a4c80ae commit 91cf607
Show file tree
Hide file tree
Showing 10 changed files with 169 additions and 22 deletions.
22 changes: 18 additions & 4 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,19 @@
# REPO MOVE!
#
# Please update your '.pre-commit-config.yaml' files.
# The new purpose-built repository is located here: https://github.com/Kintyre/ksconf-pre-commit
#
# Support for running pre-commit directly against *this* repository will
# continue to through 2024, but will be subject to increasingly aggressive reminders ;-)
#
# Note tha the 'lxml' requirement will be dropped much sooner and therefore may require tweaking,
# so you may as well go ahead and switch repos!
#
# After v0.13, we'll sunset these hooks using the technique descried here: https://github.com/pre-commit/pre-commit/issues/2003#issuecomment-895375482
#
# name: {OLDNAME} Migrated to {REPO}
# language: fail

- id: ksconf-check
name: Ksconf Splunk CONF - Check syntax
description: Check that all .conf files are syntactically valid
Expand All @@ -13,10 +29,8 @@
files: (\.conf|(local|default)\.meta)$

- id: ksconf-xml-format
### WORST CASE: Move this into it's own ksconf-precommit repo, that intalls 'ksconf' and 'lxml'.... ;-( Probably easier than making pre-commit specific tags/branches or some junk like that...
# Apparently additional_dependencies only works on pre-commit-config not in hooks?!
#additional_dependencies: lxml (unclear if this is supported for all languages)
# I wonder what would happen if I set lxml as a build requirement instead of an install requirement. Since 'pip install .' would likely install both?!?!
# If you intend to use this, you must add: additional_dependencies: [lxml] to your .pre-commit-config.yaml
# Even better. Migrate to 'ksconf-pre-commit'!
name: Ksconf Splunk CONF - Normalize XML
description: Normalize and apply consistent XML indentation and CDATA usage for XML dashboards and navigation files.
entry: ksconf xml-format -q
Expand Down
21 changes: 21 additions & 0 deletions docs/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,27 @@ Changelog
.. note:: Changes in the *devel* branch, but not released yet are marked as *DRAFT*.


Ksconf 0.12
-----------

**Highlights:**

* Pre-commit hooks have been moved into their own `ksconf pre-commit repo <ksconf-pre-commit>`_ repository.
To allow time for migration to the new repo, the existing hooks will remain for a few release before being removed.
To migrate, simply add ``-pre-commit`` to the end of the ``repo`` field, and update ``rev`` to ``v0.12.0`` or later.
* Dropped hard ``lxml`` from requirements.
This is still handled automatically when using the pre-commit hooks (from the new repository).
But this may be missing.
To get access to all CLI functionality, run ``pip install ksconf[thirdparty]``,
or for the full experience use ``pip install ksconf[fully-loaded]``


Ksconf v0.12.0 (DRAFT)
~~~~~~~~~~~~~~~~~~~~~~~~~~~

* Drop ``lxml`` dependency.
* Moved pre-commit hooks to `ksconf pre-commit repo <ksconf-pre-commit>`_, and started deprecation of the hooks in the main `ksconf repo <ksconf>`_ repo.


Ksconf 0.11
-----------
Expand Down
5 changes: 2 additions & 3 deletions docs/source/cmd_xmlformat.rst
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ Specific awareness of various Simple XML tags is baked into this product.
(2) already ships with Splunk's embedded Python.

This is an optional requirement, unless you want to use the ``xml-format`` command.
However, due to packaging limitations and pre-commit hook support, installation of the python package
will result in an attempt to install lxml as well.
Please :ref:`reach out <contact_us>` if this is causing issues for you; I'm looking into other options too.

As of v0.12.0, this is not longer installed by the ``ksconf`` package.
However, if you are using pre-commit hooks from the `ksconf-pre-commit repo`_ for the ``ksconf-xml-format`` hook.



Expand Down
1 change: 1 addition & 0 deletions docs/source/common
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
.. _gitlint: https://jorisroovers.github.io/gitlint/
.. _ksconf app for splunk: https://splunkbase.splunk.com/app/4383/
.. _ksconf repo: https://github.com/Kintyre/ksconf
.. _ksconf-pre-commit repo: https://github.com/Kintyre/ksconf-pre-commit
.. _ksconf-wheel: https://pypi.org/project/ksconf/#files
.. _ksconf: https://pypi.org/project/ksconf
.. _pluggy: https://pluggy.readthedocs.io/en/latest/
Expand Down
8 changes: 4 additions & 4 deletions docs/source/contrib.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ The ksconf project uses the pre-commit_ hook to enable the following checks:

.. note:: Multiple uses of pre-commit

Be aware, that the `ksconf repo`_ both uses pre-commit for validation of it's own content, and it provides a pre-commit hook service definition for other repos.
Be aware, that the `ksconf repo`_ uses pre-commit for validation of it's own content, and `ksconf-pre-commit repo`_ provides a pre-commit hook service definition for other repos.
The first scenario is discussed in this section of the guide.
The second scenario is for repositories that house Splunk apps to use :ref:`ksconf_cmd_check` and :ref:`ksconf_cmd_sort`
as easy to use hooks against their own ``.conf`` files which is discussed further in :ref:`ksconf_pre_commit`.
Expand All @@ -35,7 +35,7 @@ Install:

.. code-block:: sh
sudo pip install pre-commit
pip install pre-commit
# Register the pre-commit hooks (one time setup)
cd ksconf
Expand Down Expand Up @@ -101,7 +101,7 @@ File path Description / purpose
``tests/tests/test_cli_CMD.py`` Add new unit test here
``docs/source/cmd_CMD.rst`` Command line documentation. Make sure to include the `argparse` module
``ksconf/setup_entrypoints.py`` Addd a new entrypoint line here, or the new command won't be registered
``.pre-commit-hooks.yaml`` Only needed if the new command is a command is pre-commit hook
``.pre-commit-hooks.yaml`` If a new command is applicable, add this to the `ksconf pre-commit repo`_.
``setup.py`` Update if there are any new external dependencies
``requirements.txt`` Same as above
``make_splunk_app`` If there's new dependencies that need to go into the Splunk app
Expand Down Expand Up @@ -129,7 +129,7 @@ The following example assume we're make a new command called ``asciiart``:
vim ksconf/setup_entrypoints*.py
git add kconf/setup_entrypoints.py
# Now run pre-commit to ensure that the new command is found sucessfully and is importable
# Now run pre-commit to ensure that the new command is found successfully and is importable
pre-commit
# Now go write code, tests, docs and commit ...
Expand Down
55 changes: 51 additions & 4 deletions docs/source/git.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ We suggest that you read the pre-commit docs and review this section when you ar
Hooks provided by ksconf
~~~~~~~~~~~~~~~~~~~~~~~~~

Three hooks are currently defined by the ksconf repository:

Three hooks are currently defined by the `ksconf-pre-commit repo <ksconf-pre-commit>`_ repository:

ksconf-check
Runs :ref:`ksconf_cmd_check` to perform basic validation tests against all files
Expand All @@ -43,6 +42,54 @@ Three hooks are currently defined by the ksconf repository:
to reduce the need for XML escaping, resulting in more readable source file.
By default, this hook looks at standard locations where XML views and navigation typically live.


Repository Change
~~~~~~~~~~~~~~~~~

As of October 2023 (v0.12), the ksconf pre-commit hooks have been moved into their own repository to simplify packing and dependency complexities.
This will impact users whenever upgrading their pre-commit configs to use the latest version of ksconf.
This will happen, for example, when running ``pre-commit autoupdate``.

To be clear, this change will not break any existing pre-commit configuration.
But to avoid any disruption, we suggest you start using this new repository now, while you're thinking about it.
The change is easy.


Migration Steps
+++++++++++++++

Edit your ``.pre-commit-config.yaml`` file to (1) use the new ``repo`` location, and (2) use a recent version in ``rev`` (v0.11.7+)

Replace this:

.. code-block:: yaml
- repo: https://github.com/Kintyre/ksconf
rev: v0.9.5
with this:

.. code-block:: yaml
- repo: https://github.com/Kintyre/ksconf-pre-commit
rev: v0.11.9
Alternately, you could run the following shell commands:

.. code-block:: sh
# Update pre-commit config in-place
sed -e 's~https://github.com/Kintyre/ksconf$~https://github.com/Kintyre/ksconf-pre-commit~' -i.bak .pre-commit-config.yaml
# Update to latest release
pre-commit autoupdate --repo https://github.com/Kintyre/ksconf-pre-commit
.. If you have dozens of app repos laying around, you can use a set of commands like so:
.. find . -name '.pre-commit-config.yaml' -maxdepth 2 | xargs fgrep -l https://github.com/Kintyre/ksconf | head -2 | (r=$PWD; while read cfg; do echo "$cfg"; ( cd $(dirname "$cfg") && sed -e 's~https://github.com/Kintyre/ksconf~https://github.com/Kintyre/ksconf-pre-commit~' -i.bak "$(basename $cfg)" && pre-commit autoupdate --repo https://github.com/Kintyre/ksconf-pre-commit -c "$(basename $cfg)"; ) done; )
Configuring pre-commit hooks in you repo
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand All @@ -54,7 +101,7 @@ To add ksconf pre-commit hooks to your repository, add the following content to
:name: .pre-commit-config.yaml
repos:
- repo: https://github.com/Kintyre/ksconf
- repo: https://github.com/Kintyre/ksconf-pre-commit
rev: v0.11.9
hooks:
- id: ksconf-check
Expand Down Expand Up @@ -82,7 +129,7 @@ For general reference, here's a copy of what we frequently use for our repos.
- id: detect-private-key
- id: mixed-line-ending
args: [ '--fix=lf' ]
- repo: https://github.com/Kintyre/ksconf
- repo: https://github.com/Kintyre/ksconf-pre-commit
rev: v0.11.9
hooks:
- id: ksconf-check
Expand Down
4 changes: 4 additions & 0 deletions docs/source/install.rst
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,11 @@ If you want the packages, install the "thirdparty" extras using the following co
pip install ksconf[thirdparty]
If you want *all* the goodies:

.. code-block:: sh
pip install ksconf[fully-loaded]
Other issues
Expand Down
47 changes: 47 additions & 0 deletions ksconf/commands/xmlformat.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from __future__ import absolute_import, unicode_literals

import os
from argparse import SUPPRESS
from collections import Counter

from ksconf.commands import KsconfCmd, dedent
Expand Down Expand Up @@ -59,7 +60,53 @@ def register_args(self, parser):
parser.add_argument("--quiet", "-q", default=False, action="store_true",
help="Reduce the volume of output.")

# Hidden arguments
parser.add_argument("--disable-pre-commit-migration-check",
default=False, action="store_true", help=SUPPRESS)

def pre_commit_repo_migration_warning(self, args):
r"""
Issue migration warning if (1) running hooks from the old repo (missing
arg), and (2) parent process is from pre-commit (env var).
Another workaround is to use:
.. code-block:: yaml
- repo: https://github.com/Kintyre/ksconf
rev: v0.11.8
hooks:
- id: ksconf-check
- id: ksconf-sort
exclude: logging\.conf
- id: ksconf-xml-format
args: --disable-pre-commit-migration-check
additional_dependencies: [lxml]
But honestly, isn't it just easy to add ``-pre-commit`` to the repo?
Remove this after Dec 2024 or v0.13.0
"""
# New repo uses the following config:
# entry: ksconf xml-format -q --disable-pre-commit-migration-check
# If this flag has been used, assume we're running from the correct repo
if args.disable_pre_commit_migration_check:
return

# See if pre-commit is my parent. Assume this based on env variable.
if os.environ.get("PRE_COMMIT", "") != "1":
return

from warnings import warn
warn("You appear to be using the 'ksconf-xml-format' pre-commit hook from the ksconf repo. "
"The ksconf pre-commit hooks have been moved to a new repo. "
"This configuration will stop working after v0.13.0 "
"Please update '.pre-commit-config.yaml' to use the new "
"repo: https://github.com/Kintyre/ksconf-pre-commit.git")

def run(self, args):
self.pre_commit_repo_migration_warning(args)
formatter = SplunkSimpleXmlFormatter()
# Should we read a list of conf files from STDIN?
if len(args.xml) == 1 and args.xml[0] == "-":
Expand Down
1 change: 0 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
entrypoints
splunk-sdk>=1.7.0
pluggy>=1.2.0
lxml>=4.6.5
27 changes: 21 additions & 6 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from ksconf.setup_entrypoints import get_entrypoints_setup

PRE_COMMIT = os.getenv("PRE_COMMIT")
package_name = "kintyre-splunk-conf" if os.getenv("BUILD_OLD_PACKAGE") == "1" else "ksconf"

if package_name == "ksconf":
Expand Down Expand Up @@ -57,6 +58,8 @@ def get_ver(_allow_git_fetch=True):
version = version.lstrip("v") # Tags format is v0.0.0
del gitout

# NOTE: pre-commit building complexities go away after v0.13.0

# If version is hex string, assume there's an issue (aka running from pre-commit's install)
# Pre-commit has it's own shallow clone that doesn't check out that tags we need to build the
# package. See https://github.com/pre-commit/pre-commit/issues/2610
Expand Down Expand Up @@ -129,6 +132,14 @@ def get_ver(_allow_git_fetch=True):
"""

install_requires = [req for req in open("requirements.txt") if req and not req[0] == "#"]

# Temporary hack. Remove in v0.13 and migrate to kintyre/ksconf-pre-commit
if PRE_COMMIT:
print("Build from within pre-commit detected. "
"Please switch to using the ksconf-pre-commit repo instead!")
install_requires.append("lxml>=4.6.5")


setup(name=package_name,
version=get_ver(),
Expand Down Expand Up @@ -175,20 +186,24 @@ def get_ver(_allow_git_fetch=True):
setup_requires=[
"wheel",
],
install_requires=[
"entrypoints",
"pluggy",
"lxml", # Added as a hard requirement to allow pre-commit to work out of the box
],
install_requires=install_requires,
# Wacky reason for this explained in ksconf/setup_entrypoints.py
entry_points=get_entrypoints_setup(),
# Not required, but useful.
extras_require={
"xml": ["lxml"],
"bash": ["argcomplete"],
"jinja": ["jinja2"],
"thirdparty": [
"splunk-sdk>=1.7.0"
"splunk-sdk>=1.7.0",
"lxml",
],
"fully-loaded": [
"lxml",
"argcomplete",
"jinja2",
"splunk-sdk",
]
},
include_package_data=True,
zip_safe=True
Expand Down

0 comments on commit 91cf607

Please sign in to comment.