Added recipe for htop #1598

keuv-grvl · 2016-09-15T16:02:31Z

No description provided.

conda-forge-linter · 2016-09-15T16:02:37Z

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 (recipes/htop) and found some lint.

Here's what I've got...

For recipes/htop:

The recipe could do with some maintainers listed in the extra/recipe-maintainers section.

conda-forge-linter · 2016-09-15T16:05:37Z

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 (recipes/htop) and found it was in an excellent condition.

jakirkham · 2016-09-15T16:29:24Z

Seems like a nice recipe to have. What is the reason for closing? Please let us know if there is anything we can do.

Normally I use glances for this as it works everywhere including Windows. Though htop is still definitely a nice utility to have.

keuv-grvl · 2016-09-15T18:41:49Z

Ow! Closing the PR was non intended.

Nonetheless, my local test (a simple conda build recipes/htop && conda install htop --use-local in an empty conda-env) do not work if conda get ncurses from conda-forge channel (5.9-9). But it works with the default channel (ncurses 5.9-8).
Any idea why?

jakirkham · 2016-09-15T18:48:53Z

Ok, cool. Just wanted to make sure you hadn't lost hope. 😄

Hmm...so ncurses build number 9 of 5.9 dropped the argument --with-terminfo-dirs. Please see PR ( conda-forge/ncurses-feedstock#19 ) for details. That had to be dropped as it caused issues ( conda-forge/ncurses-feedstock#17 ) for other users. We could try to bring it back with a list of directories to search.

Could you please provide info about your system? Guessing Linux of some kind?

keuv-grvl · 2016-09-16T07:38:25Z

I use Ubuntu 16.04.1 x86_64 with conda 4.2.6.

I'm not familiar with ncurses, but maybe this and this could help.

jakirkham · 2016-09-18T16:11:25Z

Yep, that seems right. Could you please submit a PR to the feedstock?

jakirkham · 2016-09-19T14:38:10Z

recipes/htop/meta.yaml

+
+source:
+  git_url: https://github.com/hishamhm/htop.git
+  git_rev: {{ version }}


Could we please switch this to a tarball instead?

fn: {{ name }}-{{ version }}.tar.gz url: https://github.com/hishamhm/htop/archive/{{ version }}.tar.gz sha256: <some checksum>

jakirkham · 2016-09-19T14:38:36Z

recipes/htop/meta.yaml

+requirements:
+  build:
+    - automake
+    - gcc


Please drop gcc as we already include the compilers on the VMs.

jakirkham · 2016-09-19T14:38:55Z

recipes/htop/meta.yaml

+  skip: true  # [win]
+
+requirements:
+  build:


Please add toolchain to properly configure the VM compilers.

jakirkham · 2016-09-19T14:39:59Z

recipes/htop/meta.yaml

+    - automake
+    - gcc
+    - libtool
+    - ncurses


Please pin to 5.9* to match our global pinnings.

Also shouldn't this be a run time dependency too?

I'm not sure if I pinned it right...

Now added to run time dependency

jakirkham · 2016-09-19T14:40:23Z

recipes/htop/meta.yaml

+
+about:
+  home: https://github.com/hishamhm/htop/
+  license: GNU General Public License, version 2 (GPL-2.0)


Please shorten to GPL-2 or similar.

Also please include the license file by adding license_file: COPYING.

jakirkham · 2016-09-19T14:41:28Z

recipes/htop/meta.yaml

+
+extra:
+  recipe-maintainers:
+    - keuv-grvl


nit: Please add terminal newline.

jakirkham · 2016-09-19T14:42:11Z

recipes/htop/meta.yaml

+  build:
+    - automake
+    - gcc
+    - libtool


Should we add pkg-config? Seems like we can't find the headers for ncurses. So this may help.

It did not change anything on my system, so I did not add it. But let met know if I have to.

jakirkham · 2016-09-19T14:42:32Z

recipes/htop/build.sh

+set -euo pipefail
+./autogen.sh
+./configure --prefix=$PREFIX
+make


Is there a make check that we can run?

There is one, but it does not seem to check something. I added it anyway.

jakirkham · 2016-09-19T14:57:40Z

recipes/htop/meta.yaml

+    - automake
+    - gcc
+    - libtool
+    - ncurses


Also shouldn't this be a run time dependency too?

jakirkham · 2016-09-19T15:09:11Z

recipes/htop/build.sh

@@ -0,0 +1,6 @@
+#!/bin/bash
+set -euo pipefail


Please add the following to fix the include failure.

export CFLAGS="-I${PREFIX}/include ${CFLAGS}"

jakirkham · 2016-09-19T15:36:42Z

Given the changes mentioned above and your PR ( conda-forge/ncurses-feedstock#23 ), I was able to build htop and run it locally. So I think we are on the right path. 😄

conda-forge-linter · 2016-09-19T17:01:26Z

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

I was trying to look for recipes to lint for you, but it appears we have a merge conflict.
Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

keuv-grvl · 2016-09-19T17:20:18Z

Thank your @jakirkham for your review!

I added your suggestions to the current PR, and it works on my machine using:

conda build recipes/htop
conda search htop --use-local
conda install htop --use-local

Let me know if you can reproduce it.
Also, I did not use conda-forge/ncurses-feedstock#23 but the current ncurses from conda-forge (5.9-9).

The odd thing is: I dropped conda-forge from my channels (using conda config --remove-key channels) to use only channel defaults, then tried to build the package, and it worked (I had to remove CFLAGS and toolchain). These packages were used as dependencies:

libgcc-5.2.0-0
libtool-2.4.2-0
m4-1.4.17-0
autoconf-2.69-0
ncurses-5.9-8

So I guess ncurses-5.9-9 from the conda-forge is "breaking" something, but I can't figure out how precisely...
Anyway, we are making some progress 😃

jakirkham · 2016-09-19T17:40:35Z

recipes/htop/COPYING

@@ -0,0 +1,355 @@
+        GNU GENERAL PUBLIC LICENSE


We shouldn't need this right? We are just grabbing it from the source code.

So which license file should I add?

We shouldn't need to add one.

The feedstock will get a BSD 3-Clause license as it is the standard.

Adding the license_file portion in the recipe was the only thing we needed to do. However, it pulls the license file from the htop's source. So it isn't using this file either.

jakirkham · 2016-09-19T17:41:34Z

recipes/htop/meta.yaml

+    - toolchain
+    - automake
+    - libtool
+    - ncurses >=5.9


This should be ncurses 5.9*. Otherwise this will break when ncurses 6.0 is released here.

jakirkham · 2016-09-19T17:41:41Z

recipes/htop/meta.yaml

+    - ncurses >=5.9
+
+  run:
+    - ncurses >=5.9


Same as above.

conda-forge-linter · 2016-09-19T17:44:14Z

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 (recipes/htop) and found it was in an excellent condition.

jakirkham · 2016-09-19T18:50:12Z

To build, we normally use a Docker container. This should be easy to do locally as long as you have Docker installed. To get the build artifacts out, you may need to use PR ( #1508 ). This will drop the package in build_artefacts/linux-64. Please try installing the package on your Linux machine with conda-forge enabled and let me know how that goes.

jakirkham · 2016-09-20T04:40:57Z

So I built this fine in the Docker container and was able to use it successfully in our container and Continuum's. Not sure what is causing you build issues locally, but would recommend using our Docker container and seeing if the resulting package will work on your OS.

keuv-grvl · 2016-09-20T09:52:17Z

I successfully built htop using the Docker container. I also could install and run htop within the Docker container.
Then I added the build_artefacts/ folder to my channels in the ~/.condarc file and I could install htop from this local channel.

Everything seems to work fine :)

So is conda-forge/ncurses-feedstock#23 still relevant?

jakirkham · 2016-09-20T13:49:02Z

Great! Glad this works for you. 😄

We can take a closer look at the build issues you encountered outside the Docker image in an issue at the feedstock. Just make sure to ping me as I'm often unsubscribed from feedstocks to keep notifications manageable.

So is conda-forge/ncurses-feedstock#23 still relevant?

No, it sounds like it isn't. Would you mind closing it and making a note about this?

jakirkham · 2016-09-20T13:52:51Z

Could you please revert/remove commit ( c3ef0b8 ) from the PR?

Edit: Also please drop the license file.

This reverts commit c3ef0b8.

jakirkham · 2016-09-20T17:58:11Z

Thanks @keuv-grvl.

jakirkham · 2016-09-20T17:59:27Z

Soon you should get an email that invite you to join conda-forge, @keuv-grvl. Once accepted you will be added to a team with the same name as this recipe. Those will give you permissions on the feedstock (repo) for this recipe.

Make sure when proposing any change that you go through the typical GitHub workflow of forking the feedstock and making changes in your fork that you PR back. Once merged CIs will build and deploy any changes you make.

Please let us know if you have any questions and welcome to conda-forge. 😄

keuv-grvl · 2016-09-21T07:14:12Z

htop was my first recipe. So thank you @jakirkham for your precious help.

Conda recipe for htop

901d0c1

add recipe maintainers

bdaf41c

keuv-grvl closed this Sep 15, 2016

keuv-grvl reopened this Sep 15, 2016

jakirkham reviewed Sep 19, 2016

View reviewed changes

keuv-grvl added 8 commits September 19, 2016 17:48

Add license file, same as htop

3f308ae

add CFLAGS

eb8cc00

add 'make check'

83ad7e9

switch to htop release instead of git revision

18cad9d

add 'toolchain' as build requirement

7849cbf

drop gcc

c7d11c5

specify ncurses version

5fb8521

newline at end of file

1ac52a0

jakirkham reviewed Sep 19, 2016

View reviewed changes

ncurses 5.9* (not 6.*)

42691ee

apply [#1508](#1508) to run Docker build

c3ef0b8

keuv-grvl added 2 commits September 20, 2016 16:03

Revert "apply [#1508](#1508) to run Docker build"

e215f7f

This reverts commit c3ef0b8.

remove useless license file

642caaa

keuv-grvl mentioned this pull request Sep 20, 2016

Specify terminfo dirs conda-forge/ncurses-feedstock#23

Closed

jakirkham merged commit 1f3698b into conda-forge:master Sep 20, 2016

keuv-grvl mentioned this pull request Sep 21, 2016

Force ncurses update while installing htop? conda-forge/htop-feedstock#1

Closed

Added recipe for htop #1598

Added recipe for htop #1598

Conversation

keuv-grvl commented Sep 15, 2016

conda-forge-linter commented Sep 15, 2016

conda-forge-linter commented Sep 15, 2016

jakirkham commented Sep 15, 2016

keuv-grvl commented Sep 15, 2016

jakirkham commented Sep 15, 2016

keuv-grvl commented Sep 16, 2016

jakirkham commented Sep 18, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Sep 19, 2016 • edited Loading

conda-forge-linter commented Sep 19, 2016

keuv-grvl commented Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

conda-forge-linter commented Sep 19, 2016

jakirkham commented Sep 19, 2016

jakirkham commented Sep 20, 2016

keuv-grvl commented Sep 20, 2016

jakirkham commented Sep 20, 2016

jakirkham commented Sep 20, 2016 • edited Loading

jakirkham commented Sep 20, 2016

jakirkham commented Sep 20, 2016 • edited Loading

keuv-grvl commented Sep 21, 2016

jakirkham commented Sep 19, 2016 •

edited

Loading

keuv-grvl commented Sep 19, 2016 •

edited

Loading

jakirkham commented Sep 20, 2016 •

edited

Loading

jakirkham commented Sep 20, 2016 •

edited

Loading