Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add current version constant #314

Closed
markhalliwell opened this issue Mar 18, 2018 · 5 comments
Closed

Add current version constant #314

markhalliwell opened this issue Mar 18, 2018 · 5 comments

Comments

@markhalliwell
Copy link
Contributor

markhalliwell commented Mar 18, 2018

Hi again.

So I've been working on https://www.drupal.org/project/markdown/issues/2952435 and have come across a slight issue 😄

In CMS systems like Drupal, there is no reliable way to determine which version of a package Composer installed during runtime (primarily due to any number of arbitrary Composer setups/workflows).

In other libraries, we're able to easily determine the library's version from a constant, e.g.:
https://github.com/michelf/php-markdown/blob/lib/Michelf/Markdown.php#L21
https://github.com/erusev/parsedown/blob/master/Parsedown.php#L20

Thus, when creating a @MarkdownParser Drupal plugin, retrieving its version is rather simple for these libraries:

  /**
   * {@inheritdoc}
   */
  public function getVersion() {
    return Parsedown::version;
  }

However, this project doesn't have a constant and the only way I've been able to (semi-)reliably retrieve this is by doing something like the following (which is hella-expensive):

  /**
   * {@inheritdoc}
   */
  public function getVersion() {
    $reflector = new \ReflectionClass(CommonMarkConverter::class);
    $path = dirname(dirname($reflector->getFileName()));
    $changelog = file_get_contents("$path/CHANGELOG.md");
    preg_match('/\[(\d+.\d+.\d+)\]/', $changelog, $matches);
    return $matches && $matches[1];
  }

Obviously, we'll have to support this for current versions until we can set a minimum requirement that includes this constant.

Thoughts?

@GrahamCampbell
Copy link
Member

The most reliable way to determine a package's version is to look in the vendor folder at the json files that composer generates. Here's some example code: https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/v2.10.4/src/ToolInfo.php.

@colinodell
Copy link
Member

I'm not strongly in favor of this, and it does require some extra work during the release process, but I can see how it would be helpful to the Drupal module so I'll add it. We can always re-evaluate later if needed :)

@colinodell
Copy link
Member

This has been implemented in 0.17.1

@markhalliwell
Copy link
Contributor Author

markhalliwell commented Mar 18, 2018

The most reliable way to determine a package's version is to look in the vendor folder at the json files that composer generates.

I thought about that as well because that would make the most logical sense, sure.

Unfortunately, in Drupal, there is no reliable way to determine exactly where the vendor's composer.json file actually lives. It could be located in any number of different locations. There's lots history on this... and I don't have time to explain it all 😃

It also doesn't help that once DrupalKernel has booted, Composer's autoloader is no longer publically available (to prevent proper workflow/API circumvention... and even more history). Attempting to a similar above approach with the autoloader is near futile.

Regardless, even if either was easily discoverable, it would also still be an additional file read + parsing. This can still be expensive depending on how the server is setup or how large the file is that is being parsed.

I'm not strongly in favor of this, and it does require some extra work during the release process

Yes, I too am generally not a fan of this, but considering that Composer's view towards versions differs dramatically to that of something like NPM, there's really no other reliable way IMO.

Besides, as stated above, it's far more efficient to use an already loaded class constant than to hunt and peck for it in a file that has to be parsed.

This has been implemented in 0.17.1

Yay! Thank you @colinodell! This makes life much easier in the Drupal world 😃

@markhalliwell
Copy link
Contributor Author

I forgot to mention that in some Drupal setups, caching and other performance optimizations can also play a huge factor into this as well.

@colinodell colinodell mentioned this issue Apr 18, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants