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

Support output format for list option #448

Merged
merged 1 commit into from
Jun 28, 2022

Conversation

cronik
Copy link
Contributor

@cronik cronik commented Jun 20, 2022

Direct cli user messages to standard error rather than standard out. Standard out
is reserved for primary output, which in cases like --list --output yaml would
be the yaml formatted plugin list. This separation of output is consistent with
general cli guidelines, https://clig.dev/#the-basics.

Fixes: #377
Fixes #284
Fixes #227

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

lgtm, can you check the test failure please?

@timja timja added the enhancement New feature or request label Jun 21, 2022
@timja
Copy link
Member

timja commented Jun 21, 2022

How does this fix #227?

@cronik
Copy link
Contributor Author

cronik commented Jun 21, 2022

With this changeset the following command can be used to generate a plugin file with the current set of effective plugins.

jenkins-plugin-cli --list --output yaml > plugins.yaml

not sure if there is more to #227 that I'm missing or not aware of.

@cronik cronik force-pushed the feature/list-output-format branch 3 times, most recently from ddd7b54 to f86feb9 Compare June 21, 2022 18:52
@cronik cronik marked this pull request as ready for review June 21, 2022 20:57
@timja
Copy link
Member

timja commented Jun 22, 2022

That issue is about generating a plugin list from an existing Jenkins home.

Currently the CLI doesn't know anything about the Jenkins home only the war file, so if I run e.g. this:

java -jar plugin-management-cli/target/jenkins-plugin-manager-2.12.7-SNAPSHOT.jar --list --output txt  -d plugins --war /Users/timja/code/jenkins/jenkins/war/target/jenkins.war

I get:

Installed plugins:
-none-

Bundled plugins:
-none-

All requested plugins:
-none-

Plugins that will be downloaded:
-none-


Done

@timja
Copy link
Member

timja commented Jun 22, 2022

While this PR looks great I don't think it fixes any of the linked issues.

It does fix an issue where in edge cases something gets printed to stdout and breaks the file.

e.g. a recent issue that fixed the single case:
#438

@MarkEWaite
Copy link
Contributor

@timja this pull request works in at least one of the use cases that I believe was previously missing from the tool. I have a directory that contains plugin files but don't have a plugins.txt file that describes those plugins and their precise versions. With this pull request, I can now run the tool and generate a plugins.txt file based on the contents of that plugins directory.

Example use case looks like this:

$ git clone -b lts-with-plugins https://github.com/MarkEWaite/docker-lfs.git
$ cd docker-lfs/ref
$ wget https://home.markwaite.net/~mwaite/jenkins-plugin-manager-2.12.7-pre.jar
$ java -jar jenkins-plugin-manager-2.12.7-pre.jar -d plugins --jenkins-version 2.332.3 --list > plugins.txt
$ cat plugins.txt

I thought that was the use case described by #227, but may have missed something in my understanding

@timja
Copy link
Member

timja commented Jun 22, 2022

Oh cool! That worked, checking again

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

I think this should be documented in the README, especially that you need to redirect stderr to dev null

Is it worth adding an option to suppress stderr except on error? quiet? Can file as a separate issue.

Sorting the result would be great too (also could be a separate pull request)

Direct cli user messages to standard error rather than standard out. Standard out
is reserved for primary output, which in cases like `--list --output yaml` would
be the yaml formatted plugin list. This separation of output is consistent with
general cli guidelines, https://clig.dev/#the-basics.

Fixes: jenkinsci#377 jenkinsci#284 jenkinsci#227
@cronik cronik force-pushed the feature/list-output-format branch from f86feb9 to d95423e Compare June 28, 2022 00:45
@timja timja merged commit 29c4aa8 into jenkinsci:master Jun 28, 2022
nafets227 added a commit to nafets227/jenkins-lts-custom that referenced this pull request Jul 26, 2022
jenkinsci/plugin-installation-manager-tool#448
introduced a clean separation of output on stdout and messaged on stderr
We will ignore the messages on stderr (they will be visible in the log)
and only use stdout to store in plugins.txt and subsequently install.
nafets227 added a commit to nafets227/jenkins-lts-custom that referenced this pull request Jul 26, 2022
jenkinsci/plugin-installation-manager-tool#448
introduced a clean separation of output on stdout and messages on stderr
We will ignore the messages on stderr (they will be visible in the log)
and use stdout to store in plugins.yaml and subsequently install.

switching to plugins.yaml replacing plugins.txt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants