Add workrave package and its dependencies by pozulp · Pull Request #7753 · spack/spack

pozulp · 2018-04-13T18:57:27Z

Workrave is a program that assists in the recovery and prevention of Repetitive Strain Injury (RSI). The program frequently alerts you to take micro-pauses, rest breaks and restricts you to your daily limit. The program runs on GNU/Linux and Microsoft Windows.

adamjstewart · 2018-04-13T19:30:41Z

var/spack/repos/builtin/packages/workrave/package.py

+    depends_on('m4',       type='build')
+
+    depends_on('libx11')
+    depends_on('py-cheetah')


Python packages are usually type=('build', 'run') dependencies.

adamjstewart · 2018-04-13T19:32:15Z

Just a few flake8 warnings:

var/spack/repos/builtin/packages/cairomm/package.py:41: [E501] line too long (86 > 79 characters)
var/spack/repos/builtin/packages/cairomm/package.py:43: [W391] blank line at end of file
var/spack/repos/builtin/packages/glibmm/package.py:37: [W293] blank line contains whitespace
var/spack/repos/builtin/packages/glibmm/package.py:51: [E501] line too long (86 > 79 characters)
var/spack/repos/builtin/packages/gtkmm/package.py:57: [E501] line too long (86 > 79 characters)
var/spack/repos/builtin/packages/pangomm/package.py:48: [E501] line too long (86 > 79 characters)
var/spack/repos/builtin/packages/py-cheetah/package.py:27: [E302] expected 2 blank lines, found 1
var/spack/repos/builtin/packages/py-cheetah/package.py:31: [E999] SyntaxError: unexpected character after line continuation character
var/spack/repos/builtin/packages/py-cheetah/package.py:32: [E111] indentation is not a multiple of four
var/spack/repos/builtin/packages/py-cheetah/package.py:32: [E113] unexpected indentation
var/spack/repos/builtin/packages/py-cheetah/package.py:33: [E122] continuation line missing indentation or outdented
var/spack/repos/builtin/packages/workrave/package.py:28: [E302] expected 2 blank lines, found 1
var/spack/repos/builtin/packages/workrave/package.py:29: [E501] line too long (82 > 79 characters)
var/spack/repos/builtin/packages/workrave/package.py:29: [W291] trailing whitespace
var/spack/repos/builtin/packages/workrave/package.py:30: [E501] line too long (83 > 79 characters)
var/spack/repos/builtin/packages/workrave/package.py:30: [W291] trailing whitespace
var/spack/repos/builtin/packages/workrave/package.py:31: [E501] line too long (87 > 79 characters)
var/spack/repos/builtin/packages/workrave/package.py:78: [W291] trailing whitespace
var/spack/repos/builtin/packages/workrave/package.py:80: [E501] line too long (81 > 79 characters)
var/spack/repos/builtin/packages/workrave/package.py:87: [E501] line too long (86 > 79 characters)

If you have flake8 installed locally, you can run spack flake8 to test if your changes worked.

pozulp · 2018-04-13T20:41:34Z

Thanks @adamjstewart for letting me know about spack flake8! I had no idea and it's really helpful! :)

adamjstewart · 2018-04-13T20:43:48Z

No problem! If you haven't seen it already, our Contribution Guide explains things like flake8 and unit tests pretty well.

pozulp · 2018-04-13T21:53:35Z

@adamjstewart, I did not know there was a Contribution Guide, thanks for letting me know!

scheibelp · 2018-04-19T20:30:56Z

var/spack/repos/builtin/packages/glibmm/package.py

+        specs = self.spec.traverse(root=False)
+        pkgconfdirs = ['%s/pkgconfig' % s.prefix.lib
+                       for s in specs if not s.external]
+        return ['PKG_CONFIG_PATH=%s' % ':'.join(pkgconfdirs)]


@mmpozulp build_environment already aggregates this into a PKG_CONFIG_PATH environment variable so you should be able to just say PKG_CONFIG_PATH=$PKG_CONFIG_PATH.

… automatically for dependencies which create a lib/pkgconfig in their install dir

pozulp · 2018-04-21T00:38:52Z

@scheibelp you're right, I committed the fix. Turns out I don't need to set PKG_CONFIG_PATH myself, and I can't figure out why I did it in the first place.

tgamblin

@mmpozulp: looks good! One last question: are the patches and other files in here available from someplace reliable? Could they be URL-fetched patches, or resources instead of included directly in Spack?

pozulp · 2018-04-23T19:30:49Z

@tgamblin Sadly, no. But here's some information for anyone from the future who happens to come across this issue.

In this PR, all of the .patch files solve compilation errors. The two .m4 files solve a configure error that manifests when these files are not present during autoreconf.
workrave

Of the .patch files and .m4 files in the workrave package dir, which are all my creation, those with URLs in comments in workrave/package.py work around bug info presented at the URL. If there is no URL in a comment for a file, then I couldn't find a corresponding documented issue. I looked in these places:

glibmm

The only .patch file in the glibmm package dir is of my creation. I couldn't find an associated bug documented anywhere. I looked in these places:

scheibelp · 2018-04-26T21:47:37Z

var/spack/repos/builtin/packages/workrave/ax_cxx_compile_stdcxx.m4

@@ -0,0 +1,980 @@
+# ===========================================================================
+#  https://www.gnu.org/software/autoconf-archive/ax_cxx_compile_stdcxx.html


@mmpozulp This looks to be available at http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_cxx_compile_stdcxx.m4;hb=579023fd0623be745c643faca62c95adab584325 as part of a git repositorygit://git.savannah.gnu.org/autoconf-archive.git. Would you be able to manage this as a resource?

Done. Updating my PR asap.

scheibelp · 2018-04-26T21:51:23Z

var/spack/repos/builtin/packages/py-cheetah/package.py

+
+    homepage = "https://pypi.python.org/pypi/Cheetah/2.4.4"
+    url      = """\
+https://pypi.python.org/packages/b4/18\


@mmpozulp If I try to visit https://files.pythonhosted.org/packages/b4/18a9b2f0f09e99024f4a9ea1ab80b6807aaecb7d1552529ea3590ad3d7bc93/Cheetah-2.3.0.tar.gz I'm getting a "file not found error", perhaps this has updated since the PR was opened?

EDIT: sorry this URL is fine, I copied it incorrectly.

Try:
https://pypi.io/packages/source/C/Cheetah/Cheetah-2.3.0.tar.gz

Ah and that URL format would be better for Spack's version substitution as well (e.g. if more versions of py-cheetah were added)

Also, there's no need to wrap long URLs onto multiple lines. We ignore URLs in our flake8 line-length checks.

I used @adamjstewart's url and made it a single line. Thanks for reminder about flake8 skipping URLs.

adamjstewart · 2018-04-26T21:59:28Z

var/spack/repos/builtin/packages/glibmm/package.py

+        """Handle glibmm's version-based custom URLs."""
+        url = "https://ftp.acc.umu.se/pub/GNOME/sources/glibmm"
+        ext = '.tar.gz' if version < Version('2.28.2') else '.tar.xz'
+        return url + "/%s/glibmm-%s%s" % (version.up_to(2), version, ext)


For all of these packages that override url_for_version, do you actually need all of these old versions where the extension differs, or do you only care about the latest release? We'll still need to override url_for_version because of the version.up_to(2), but we could drop a line from the function.

Yes, the only workrave spec that I have been successful building involves the older versions which use the .tar.xz extension. That's because I'm using my system (RHEL7) glib as an 'external', and glibmm, which is c++ wrappers for glib, needs to be approximately as old as my system glib to get past pkg-config tests of the glib version during configure.

…workrave

…ix ImportError: No module named site (issue #6968)

scheibelp · 2018-06-08T18:30:08Z

var/spack/repos/builtin/packages/workrave/package.py

+    patch('add_time_header.patch')
+
+    # removes call to missing glimm api function
+    patch('signal_size_changed.patch', when='@1.10.0')


@mmpozulp I'm having trouble finding other instances of this. As far as you know, has this only happened on your system? If so, do you have a notion of why? I'm concerned this means that something is missing or some component needs to be enabled that currently isn't

@scheibelp I'm comfortable deleting this patch. It's only necessary when building the workrave tarball that comes from sourceforge (which was what I encountered first when googling for workrave, but am no longer using). Since then I discovered a workrave repo on github. I am using the github repo as the package url, not the sourceforge url, partly because of issues like this. The sourceforge tarball src calls nonexistent functions, like Gtk::StatusIcon::property_embedded(). I looked for it in several versions of gtkmm and couldn't find it.

Got it - I will wait for that update. Are the other custom patches similarly resolved by changing the source?

No. I just tried commenting-out the other 3 patch() calls one-at-a-time and each resulted in an error:

#patch('add_time_header.patch')

>> 829 DayTimePred.cc:136:9: error: use of undeclared identifier 'localtime' 830 ret = localtime(&last_time);

#patch('dont_get_widget.patch')

>> 2631 ../../plugin/statistics/gtkmm/src/StatisticsDialog.cc:731:12: error: no member named 'get_widget_for_response' in 'Gtk::MessageDialog' 2632 mb_ask.get_widget_for_response( Gtk::RESPONSE_NO )->grab_default();

#patch('no_gettext.patch')

>> 393 config.status: error: po/Makefile.in.in was not created by intltoolize.

Thus, I will leave in the remaining 3 workrave patches.

…ourceforge workrave tarball, but not for the github releases

Patches available from URLs have been updated accordingly

scheibelp · 2018-06-22T01:25:46Z

Thanks!

Add workrave package and its dependencies

b24bcd7

adamjstewart added the new-package label Apr 13, 2018

adamjstewart reviewed Apr 13, 2018

View reviewed changes

flake8 fixes

20dfb42

scheibelp reviewed Apr 19, 2018

View reviewed changes

Don't override configure_args to set PKG_CONFIG_PATH; spack does this…

b1cf2ff

… automatically for dependencies which create a lib/pkgconfig in their install dir

tgamblin previously requested changes Apr 22, 2018

View reviewed changes

scheibelp reviewed Apr 26, 2018

View reviewed changes

adamjstewart reviewed Apr 26, 2018

View reviewed changes

pozulp added 2 commits June 4, 2018 00:20

Merge branch 'develop' of https://github.com/LLNL/spack into package/…

4d7de8f

…workrave

Make workrave m4 resource() directive calls, change py-cheetah url, f…

fef7308

…ix ImportError: No module named site (issue #6968)

scheibelp reviewed Jun 8, 2018

View reviewed changes

remove signal_size_changed.patch which was required for the ancient s…

d224bae

…ourceforge workrave tarball, but not for the github releases

scheibelp merged commit 3c627ea into spack:develop Jun 22, 2018

		@@ -0,0 +1,980 @@
		# ===========================================================================
		# https://www.gnu.org/software/autoconf-archive/ax_cxx_compile_stdcxx.html

Conversation

pozulp commented Apr 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamjstewart commented Apr 13, 2018

Uh oh!

pozulp commented Apr 13, 2018

Uh oh!

adamjstewart commented Apr 13, 2018

Uh oh!

pozulp commented Apr 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pozulp commented Apr 21, 2018

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

pozulp commented Apr 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scheibelp Apr 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

scheibelp commented Jun 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

scheibelp Apr 26, 2018 •

edited

Loading