Hi I was just wondering if there were any plans for module maintainers to implement the Libraries API style such as in the WYSIWYG project?

This post explains what was done there for files or libraries that are needed or required to run a module. It might save some work to implement a like or similar solution for the libraries that are required for the installation of this module.

Comments

easyfit’s picture

Hi and thanks for the suggestion. Libraries API seems like a good intiative, I'll think about implementing it.

lee20’s picture

Title:Any plans to implement module similiar to the libraries API?» Integrate AES with the libraries API?
Version:6.x-1.4» 7.x-1.x-dev
Status:Active» Postponed

This crossed my mind when I first installed the module so this is certainly something that will happen in the coming versions of AES.

However, I don't know if I want to require the dependency for the libraries api so I would want it to function with and without the Libraries module. I'm sure this can be done but I'm not deeply familiar with the libraries api yet.

When AES is looking for the phpseclib library it would first check for the libraries api. If it exists, then it would use the library api's functions to attempt to load the library. If the libraries api exists and fails to load the phpseclib library then AES may try to load it manually or may just fail to load it as well.

If the libraries api does not exist, it would look for phpseclib in the same location as the libraries api would. It would also check for what would become the legacy path to phpseclib which is inside the aes module folder.

Postponing for now. I am trying to decide how I am going to support the D7 branch before I start creating version 2.x branches for this module.

svendecabooter’s picture

Status:Postponed» Needs review
StatusFileSize
new608 bytes

Below is a patch to integrate AES module with the Libraries API (branch 1.x)

thimothoeye’s picture

Status:Needs review» Needs work

Patch applies cleanly on 1.5, but it would be nice if it would be able to detect the libraries path on hook_install too :)

svendecabooter’s picture

Status:Needs work» Needs review
StatusFileSize
new1.21 KB

Added also on hook_install()

thimothoeye’s picture

Status:Needs review» Reviewed & tested by the community

Applies cleanly and works as advertised!

stewart.adam’s picture

Works for me as well. Commit this please!

dpovshed’s picture

Issue summary:View changes
Status:Reviewed & tested by the community» Needs work

Idea is very nice, I tried to test it with current libraries-2.x and here is my results.

I feel the the patch is a bit incomplete. Just test the following case:

You having phpseclib under AES directory, and libraries module active. Then your code will report that there are no phpseclib at all.

Thus, all existing user's may suffer from this and require additional tuning of the website.

Can you tune up your code so we do not force users to move phpseclib if they have it working for years in old place?

Thanks!

pianomansam’s picture

Status:Needs work» Needs review
StatusFileSize
new1.3 KB

dpovshed, I've updated the patch to not only check that Libraries API is enabled but that the phpseclib library exists within the libraries folder. If it doesn't, then it looks for it within the AES folder. This way users don't necessarily have to move the library if they already have it in the AES folder.

dpovshed’s picture

Status:Needs review» Needs work

Sam, thank you for trying to help!..

However, if we want the feature to be implemented, we really need more consistent approach. Simple search across the module for 'phpsec' revealed at least two places where code rely on library presence in module directory.

see function aes_get_available_aes_implementations()
see function aes_requirements()

So implementation of the feature require more care than it may seems to not let down current users.

pianomansam’s picture

dpovshed, I'm not sure I follow what you are saying. In your comment in #8 you asked for the ability not to force users to use Libraries API if they want to keep the phpsec folder in the AES folder. My patch uses the patch in #5 but makes sure there is a phpsec in the libraries folder before trying to use Libraries API. This means that even if the user has Libraries API installed, if they want to keep the phpsec folder within the AES folder, they can.

The two functions you mention (aes_get_available_aes_implementations and aes_requirements) are the functions that my patch updates. If you look at the patch you will see this is true.

dpovshed’s picture

Status:Needs work» Needs review

@pianomansam, sorry I obviouly overlooked that you patched aes_get_available_aes_implementations() as well.

I'll review the patch shortly; status updated.

dpovshed’s picture

Status:Needs review» Fixed

@pianomansam, patch is reviewed, tested and committed to dev.

Minor suggestion: next time please check your patch to not have a trailing whitespaces; easily can be made with git apply.

---

git apply integrate-aes-with-libraries-627336-9.patch
integrate-aes-with-libraries-627336-9.patch:16: trailing whitespace.
 
warning: 1 line adds whitespace errors.

---

Thank you for your help!

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

  • Commit 506aadb on 7.x-1.x, 8.x-2.x authored by pianomansam, committed by dpovshed:
    Issue #627336 by svendecabooter, pianomansam | kewlguy: Integrate AES...