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

include manpages #65

Merged
merged 4 commits into from
Sep 11, 2019
Merged

Conversation

timsnyder
Copy link
Contributor

@timsnyder timsnyder commented Sep 9, 2019

from prebuilt kernel.org releases on [not win] platforms. This enables the --help
option on all commands.

fixes #17

Checklist

  • Used a fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

from prebuilt kernel.org releases.  This enables the --help
option on all commands.

fixes conda-forge#17
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

@timsnyder
Copy link
Contributor Author

@conda-forge-admin, please rerender

@timsnyder
Copy link
Contributor Author

I'm looking into the linting issue.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I tried to re-render for you, but it looks like there was nothing to do.


# Install manpages
cp -r manpages/* $PREFIX/man
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be $PREFIX/share/man?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well. That's a good question. I thought the default was $PREFIX/share/man but I looked for other packages that have manpages and saw perl is installing to $PREFIX/man. I did determine that man on my ubuntu vm will find $PREFIX/man without MANPATH needing to be set.

Another good question is should we create a recipe for man so that we can configure it to correctly find manpages from $PREFIX/<wherever>?

Copy link
Member

Choose a reason for hiding this comment

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

See man man (particularly the info on MANPATH_MAP) and look at /private/etc/man.conf. A comment from that file (on my Mac):

# If people ask for "man foo" and have "/dir/bin/foo" in their PATH
# and the docs are found in "/dir/man", then no mapping is required.

There are also some details at conda/conda#845

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you guys have the manifest from all packages indexed in some searchable location? If so, I'd be happy to look to see which packages have a man directory and where they are putting it.

If you don't have such a searchable location across all packages, I can do a more brute force grind through them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asmeurer The magical looking closely to PATH only seems to work when MANPATH is unset and you haven't passed in -M on the cmdline. Perhaps a man package could pre-pend $PREFIX/man if MANPATH is set, otherwise, leave it unset?

I think what I have in this PR is at least working better than not having the manpages at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm also realizing that the only reasons why I have MANPATH set are really ancient, predate conda and I'll be better off if I stop setting it and let man just look at PATH.

This morning, I confirmed that my system man on RHEL6 does find manpages in $PREFIX/man without MANPATH but does not find $PREFIX/share/man. I also see that there are conda packages that install manpages into $PREFIX/share/man and as I mentioned earlier, there are packages that install into $PREFIX/man

Standardizing conda-forge on a manpage install path and updating the impacted recipes seems bigger than this PR. I'd be happy to work on that. @asmeurer and @scopatz, do you guys think we could go ahead and merge this PR?

Copy link
Member

Choose a reason for hiding this comment

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

From man man:

       If you don't specify an explicit path list with -M or MANPATH, man develops its own path list based on the contents of  the  configura-
       tion  file  /private/etc/man.conf.   The MANPATH statements in the configuration file identify particular directories to include in the
       search path.

So it's by design that setting MANPATH or -M overrides the defaults (it's not like PYTHONPATH which appends the defaults).

@timsnyder
Copy link
Contributor Author

Looking at the recipe linting issue, this is the traceback:

(base) tsnyder:~/git-feedstock (17_include_manpages) $ conda smithy recipe-lint recipe
Traceback (most recent call last):
  File "/home/ubuntu/miniconda3/bin/conda-smithy", line 10, in <module>
    sys.exit(main())
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/conda_smithy/cli.py", line 470, in main
    args.subcommand_func(args)
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/conda_smithy/cli.py", line 365, in __call__
    return_hints=True,
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/conda_smithy/lint_recipe.py", line 573, in main
    meta = yaml.load(content)
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/main.py", line 341, in load
    return constructor.get_single_data()
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
    node = self.composer.get_single_node()
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/composer.py", line 78, in get_single_node
    document = self.compose_document()
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/composer.py", line 101, in compose_document
    node = self.compose_node(None, None)
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/composer.py", line 138, in compose_node
    node = self.compose_mapping_node(anchor)
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/composer.py", line 218, in compose_mapping_node
    item_value = self.compose_node(node, item_key)
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/composer.py", line 136, in compose_node
    node = self.compose_sequence_node(anchor)
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/composer.py", line 179, in compose_sequence_node
    while not self.parser.check_event(SequenceEndEvent):
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/parser.py", line 140, in check_event
    self.current_event = self.state()
  File "/home/ubuntu/miniconda3/lib/python3.7/site-packages/ruamel/yaml/parser.py", line 525, in parse_block_sequence_entry
    token.start_mark,
ruamel.yaml.parser.ParserError: while parsing a block collection
  in "<unicode string>", line 8, column 3:
      - url: https://github.com/git/gi ... 
      ^ (line: 8)
expected <block end>, but found '?'
  in "<unicode string>", line 17, column 3:
      url: https://github.com/git-for- ... 
      ^ (line: 17)

This looks like the recipe-linter is trying to read the meta.yaml without doing the jinja preprocessing. Is that the intended way for the linter to work? If so, I'll need to make the Windows build look like a multi-source build even though it really isn't.

conda-smithy reads meta.yaml without applying the preprocessing selectors. For
this to work, we need to make the windows  look like a multiple-source item even though
it will only have a single source after the proprocessing selectors are applied.

Also needed to cleanup spacing before preprocessing selectors.
@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@timsnyder
Copy link
Contributor Author

I pushed another commit that makes recipe-lint happy.

@timsnyder
Copy link
Contributor Author

@conda-forge-admin, please rerender

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-webservice.

I tried to re-render for you, but it looks like there was nothing to do.

@timsnyder
Copy link
Contributor Author

timsnyder commented Sep 9, 2019

The linux_aarch64_ build is failing because it looks like conda build isn't creating $PREFIX/man. I was surprised that on linux_ it seemed to already exist and I took out the mkdir -p $PREFIX/man from build.sh. Perhaps I should put it back?

Yes. I should. Looks like linux_ autoconf has perl as a dependence and that creates the $PREFIX/man directory when it is installed. I'm guessing that linux_aarch64_ and linux_ppc64le_ may not have manpages in their perl packages.

@timsnyder timsnyder mentioned this pull request Sep 9, 2019
4 tasks
not all platforms with have $PREFIX/man created by
dependencies and we shouldn't rely on the deps to create
the directory anyway.
@scopatz
Copy link
Member

scopatz commented Sep 10, 2019

@timsnyder, let us know when this is ready for review again!

@timsnyder
Copy link
Contributor Author

timsnyder commented Sep 11, 2019

@scopatz I think it's ready for re-review and merging if we're done talking about where the manpages should be installed with @asmeurer

@scopatz scopatz merged commit 4b1de47 into conda-forge:master Sep 11, 2019
@scopatz
Copy link
Member

scopatz commented Sep 11, 2019

Thanks!

@timsnyder timsnyder deleted the 17_include_manpages branch September 13, 2019 15:06
@phil-blain
Copy link
Contributor

@timsnyder, @scopatz hi! I have just done conda update git in my 'git' environment on my Mac (OS X 10.11.6) but neither man git-$cmd, git help $cmd nor git $cmd --help show the corresponding man pages (they show the system ones), although I checked they are correctly installed in $CONDA_PREFIX/man/man1. It was not clear from the conversation above, should it work on macOS ?

@jakirkham
Copy link
Member

jakirkham commented Oct 7, 2019

@phil-blain, please open a new issue.

Edit: Raised as issue ( #66 ).

@phil-blain phil-blain mentioned this pull request Oct 7, 2019
@kdpenner
Copy link

I just ran condo install git and the manpages weren't installed.

@jakirkham
Copy link
Member

Please raise a new issue

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.

missing git manpages
8 participants