pkgconfig for gmime correct?

[c-alpha]

https://github.com/astroidmail/homebrew-astroid/blob/b252d86f408c7645840b86a591df0d1937e8210d/Formula/astroid.rb#L28

For me, brew install notmuch put it in a location of its own, not under notmuch. So for me the location of the gmime pkgconfig is /usr/local/opt/gmime/lib/pkgconfig.

Is that a special case for me using Homebrew, or should this be updated?

[c-alpha]

Actually, I think there might be a different issue. I have gmime 3.0.2 installed via homebrew (as a result of installing notmuch). The astroid homebrew build fails on not finding gmime 2.6 however:

Checking for gmime-2.6 >= 2.6.18... no gmime-2.6 not found.

This seems to come from the top-level SConstruct? Looking at lines 195 through 203 of that:

mime_version = ''
if conf.CheckPKG ('gmime-3.0 >= 3.0.0'):
  env.ParseConfig ('pkg-config --libs --cflags gmime-3.0')
  gmime_version = '3.0'

elif conf.CheckPKG ('gmime-2.6 >= 2.6.18'):
  env.ParseConfig ('pkg-config --libs --cflags gmime-2.6')
  gmime_version = '2.6'
  print ("warning: gmime-2.6 will not be supported in the future.")

This would seem ok to me. Running pkg-config --libs --cflags gmime-3.0 at a shell prompt produces the expected results. Why is it insisting on gmime 2.6 then?

[gauteh]

This is probably a relic from gmime 2 to gmime 3. Now both astroid and notmuch support both gmimes, the important point is that they both use the same version. This just ensures that astroid uses the gmime shipped with notmuch -- but I don't know if the notmuch package still does that.

[c-alpha]

This just ensures that astroid uses the gmime shipped with notmuch -- but I don't know if the notmuch package still does that.

No it doesn't ship it anymore. The notmuch formula names gmime as a dependency, and it gets a standard install. IMO you could now drop any special gmime handling and simply assume the right version is there.

Odd thing is though, given the above quoted if/elif construct, I would have expected scons to check for gmime-3.0 first. But it seems to go to gmime-2.6 directly just breaks if it can't find that:

scons: Reading SConscript files ... building release: v0.9.1 debug flag enabled: False Checking for pkg-config... yes Checking for gtkmm-3.0 >= 3.10... yes Checking for glibmm-2.4... yes Checking for gmime-2.6 >= 2.6.18... no gmime-2.6 not found.

If reporting this issue please do so at (not Homebrew/brew or Homebrew/core): [...]

[c-alpha]

Not knowledgeable about python at all, but maybe it's the empty lines 199 and 204 (i.e. those separating the if, elif, and else?

[gauteh]

Seems like you're building a pre-gmime3 release. Update version to v0.10.1.

[gauteh]

You're interpreting the python correctly, its just that that's a later version.

[c-alpha]

Thanks for the version hint!

I couldn't tweak the formula however, since accessing https://github.com/astroidmail/astroid/archive/v0.10.0.tar.gz yields a 404.

The --release argument didn't cut it either:

$ brew install astroid --release=0.10.0
==> Installing astroid from astroidmail/astroid
Warning: astroidmail/astroid/astroid: this formula has no --release= option so it will be ignored!

So I went for --HEAD which finally starts compiling astroid itself, but again breaks with an error:

compiling ==> src/utils/gmime/gmime-filter-html-bq.c
In file included from src/utils/gmime/gmime-filter-html-bq.c:35:
src/utils/compiler.h:5:27: error: missing ')' after 'clang'
# if __has_attribute(clang::fallthrough)
                     ~~~~~^
src/utils/compiler.h:5:21: note: to match this '('
# if __has_attribute(clang::fallthrough)
                    ^
1 error generated.

Which IMO, and according to the clang docs at https://clang.llvm.org/docs/LanguageExtensions.html#has-cpp-attribute, should use __has_cpp_attribute instead of __has_attribute.

In summary, I think I would be good to have two things:

1. minimally update the readme to hint to the fact that --HEAD should be built, if not updating the formula itself;

2. fix the clang branch in astroid/src/utils/compiler.h (I'm happy to submit a pull request if that helps)

[gauteh]

c-alpha writes on oktober 3, 2017 17:52:

Thanks for the version hint!

I couldn't tweak the formula however, since accessing https://github.com/astroidmail/astroid/archive/v0.10.0.tar.gz yields a 404.

Is the URL wrong?

The --release argument didn't cut it either:

$ brew install astroid --release=0.10.0
==> Installing astroid from astroidmail/astroid
Warning: astroidmail/astroid/astroid: this formula has no --release= option so it will be ignored!

So I went for --HEAD which finally starts compiling astroid itself, but again breaks with an error:

compiling ==> src/utils/gmime/gmime-filter-html-bq.c
In file included from src/utils/gmime/gmime-filter-html-bq.c:35:
src/utils/compiler.h:5:27: error: missing ')' after 'clang'
# if __has_attribute(clang::fallthrough)
                     ~~~~~^
src/utils/compiler.h:5:21: note: to match this '('
# if __has_attribute(clang::fallthrough)
                    ^
1 error generated.

Which IMO, and according to the clang docs at https://clang.llvm.org/docs/LanguageExtensions.html#has-cpp-attribute, should use __has_cpp_attribute instead of __has_attribute.

In summary, I think I would be good to have two things:

1. minimally update the readme to hint to the fact that --HEAD should be built, if not updating the formula itself;

That would probably be the correct thing to do.

1. fix the clang branch in astroid/src/utils/compiler.h (I'm happy to submit a pull request if that helps)

Yes, but this is C code. If this is still correct, please send a PR.

[c-alpha]

The good news first: I got it to compile and run. Hooray!

I couldn't tweak the formula however, since accessing https://github.com/astroidmail/astroid/archive/v0.10.0.tar.gz yields a 404.

Is the URL wrong?

In the tap formula, it says:

url "https://github.com/astroidmail/astroid/archive/v#{version}.tar.gz"

So I did a wget for

https://github.com/astroidmail/astroid/archive/v0.9.1.tar.gz

which worked fine. But a wget for

https://github.com/astroidmail/astroid/archive/v0.10.0.tar.gz

fails with a 404. So I can't just download the formula, bump up the version, and install from the local formula.

1. fix the clang branch in astroid/src/utils/compiler.h (I'm happy to submit a pull request if that helps)

Yes, but this is C code. If this is still correct, please send a PR.

Done. astroidmail/astroid#413

1. minimally update the readme to hint to the fact that --HEAD should be built, if not updating the formula itself;

That would probably be the correct thing to do.

As soon as the PR gets merged, brew install astroid --HEAD should work out of the box again.

[gauteh]

Yey!

[gauteh]

I'm guessing this one is fixed by now?

[c-alpha]

I'm guessing this one is fixed by now?

'Tis.