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

updates how accumulo version is obtained #304

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

keith-turner
Copy link
Contributor

Uses maven to get accumulo version from pom rather than xmllint. These changes will work with apache/accumulo#4930

Uses maven to get accumulo version from pom rather than xmllint.  These
changes will work with apache/accumulo#4930
Copy link
Contributor

@ddanielr ddanielr left a comment

Choose a reason for hiding this comment

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

using a mvn plugin seems less prone to risk vs a piped grep & cut command

@keith-turner keith-turner merged commit bb89e15 into apache:main Oct 3, 2024
1 check passed
@ctubbsii
Copy link
Member

I don't have an issue with these changes, but wanted to add some extra info for context:

using a mvn plugin seems less prone to risk vs a piped grep & cut command

The risks of errors are very minimal. All the commands used are standard with modern linux distros. The biggest risk is getting xmllint from libxml instead of libxml2. Both have the --shell option to do xpath expressions, but only libxml2 has the --xpath option which would have made the command much cleaner:

xmllint pom.xml --xpath '/*[local-name()="project"]/*[local-name()="version"]/text()'

The xpath expression could be further simplified (in either --shell or --xpath) if the xml elements weren't in the default namespace. Unfortunately, if you define a default namespace, xpath has no way of identifying elements explicitly, so you have to use the local-name() matchers. But if you strip off the xmlns= that defines the default namespace for elements, either by removing it or giving it a temporary name, you can greatly simplify the xpath expression:

xmllint --xpath '//project/version/text()' <(sed 's/xmlns=/xmlns:bak=/' pom.xml)

All of these options (including the original) are blazing fast, much faster than using Maven. The last one would be my preferred approach, since it's the simplest to maintain and is super fast. Unfortunately, some distros don't have libxml2 yet and are still using libxml, so xmllint is too old to have the --xpath option.

The Maven approach is slow, but it's nice because it's really portable and easy to maintain. It can also evaluate Maven properties, rather than just parse the XML values. We don't need that here, but it's nice to have an example to follow if we need to do something like it in the future. It seems like this feature might have been added in Maven 3.1 (well, the forceStdout option, to make it useful, was added then, anyway).

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.

3 participants