Improve the C plugin tutorial and fix the property issue

Review Request #1707 — Created Aug. 31, 2022 and updated

Information

pidgin/pidgin
default

Reviewers

Improve the C plugin tutorial and fix the property issue


 
Summary ID
Add missing property to hello world plugin
571fc7f8b6d8a46a0954fa239d1a5aa5224c0caf
Add compiling section to the C plugin tutorial
72aa8341d272048950d3ad41665ea4f7ab78dc9b
Correct the title, Fix the indentation of the example, Add `setup' to `meson build' as it's ambiguous and deprecated.
6bb94f5c9f277330e4891ff2cbf3e80d8a13ccb8
Allow the user to change the installation directory
600624a6032071fbc8bbc7cee2e4e567c44a3de4
Description From Last Updated

This should probably be rephrased to something like "stand alone build system" or maybe just "building" or something. In purple3/pidgin3 …

grimgrim

While this is generally good for users, it causes all sorts of issues for packagers. Perviously plugins have checked if …

grimgrim

indentation is weird here

grimgrim

install directory should be configured via the --prefix argument to meson which is available via get_option('prefix'). There's no need to …

grimgrim
grim
  1. 
      
  2. This should probably be rephrased to something like "stand alone build system" or maybe just "building" or something. In purple3/pidgin3 we explicitly don't support the old setup where you could drop a plugin file in the directory and then compile it there which I believe is what you're referncing.

  3. While this is generally good for users, it causes all sorts of issues for packagers. Perviously plugins have checked if a custom prefix was set and if not then fall back to the value in the plugin directory.

    I know this sounds silly, but this is a real issue for homebrew on mac and something else I can't recall right now.

    1. Packagers can set $DESTDIT to install it elsewhere I think.

    2. Setting DESTDIR as a package doesn't fix the issue with homebrew though.

      grim-catalina:~ garykramlich$ pkg-config --variable=plugindir pidgin
      /usr/local/Cellar/pidgin/2.14.10_2/lib/pidgin
      

      Pidgin actually looks in /usr/local/lib/pidgin in homebrew for plugins not the directory that package config provides.

      There was some other reasoning, way back when, that DESTDIR didn't fix, which is why we recommned just installing into the prefix that was specified to the plugin's build system.

    3. This issue still remains.

  4. indentation is weird here

  5. 
      
grim
  1. 
      
  2. This has been sitting for 6 weeks now. Please let me know if you would like to address the issues or if this should be dropped.

  3. 
      
goodspeed
goodspeed
Review request changed

Commits:

Summary ID
Add missing property to hello world plugin
08f763f8d0175a112f42034c50c69f6be550fa58
Add compiling section to the C plugin tutorial
bfb0d1b92ea922bf850b2649f4004a2d5afc0ad9
Correct the title, Fix the indentation of the example, Add `setup' to `meson build' as it's ambiguous and deprecated.
0f8d2a1fac4bd60188cf28e4bac44d1e08dd854d
Add missing property to hello world plugin
571fc7f8b6d8a46a0954fa239d1a5aa5224c0caf
Add compiling section to the C plugin tutorial
72aa8341d272048950d3ad41665ea4f7ab78dc9b
Correct the title, Fix the indentation of the example, Add `setup' to `meson build' as it's ambiguous and deprecated.
6bb94f5c9f277330e4891ff2cbf3e80d8a13ccb8
Allow the user to change the installation directory
600624a6032071fbc8bbc7cee2e4e567c44a3de4

Diff:

Revision 3 (+94 -6)

Show changes

grim
  1. 
      
  2. doc/reference/libpurple/tut_c_plugins.md (Diff revisions 2 - 3)
     
     

    install directory should be configured via the --prefix argument to meson which is available via get_option('prefix').

    There's no need to pull a shell environment variable in here which would break the build when not using a UNIX shell, like compiling under command prompt or power shell on windows.

    Generally speaking, if the prefix is set to /usr/local it should be save to override with the value from purple's pkg-config file.

    1. Forgot to mention, sorry for the late review, some how I didn't notice that there was an update here.

  3. 
      
Loading...