Improve the C plugin tutorial and fix the property issue

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

goodspeed
pidgin/pidgin
default
pidgin

Improve the C plugin tutorial and fix the property issue



Summary
Add missing property to hello world plugin
Add compiling section to the C plugin tutorial
Correct the title, Fix the indentation of the example, Add `setup' to `meson build' as it's ambiguous and deprecated.
Allow the user to change the installation directory
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
-
Add missing property to hello world plugin
-
Add compiling section to the C plugin tutorial
-
Correct the title, Fix the indentation of the example, Add `setup' to `meson build' as it's ambiguous and deprecated.
+
Add missing property to hello world plugin
+
Add compiling section to the C plugin tutorial
+
Correct the title, Fix the indentation of the example, Add `setup' to `meson build' as it's ambiguous and deprecated.
+
Allow the user to change the installation directory

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...