NotImplementedError: Setting version is not implemented by RscriptRequirement

[jayvdb]

[   83s] _________________ ERROR collecting tests/r/FormatRBearTest.py __________________
[   83s] tests/r/FormatRBearTest.py:1: in <module>
[   83s]     from bears.r.FormatRBear import FormatRBear
[   83s] bears/r/FormatRBear.py:20: in <module>
[   83s]     prerequisite_check_fail_message='Please install formatR for this bear '
[   83s] bears/r/FormatRBear.py:36: in FormatRBear
[   83s]     version='>1.5'),
[   83s] /usr/lib/python3.7/site-packages/dependency_management/requirements/RscriptRequirement.py:50: in __init__
[   83s]     'Setting version is not implemented by RscriptRequirement')
[   83s] E   NotImplementedError: Setting version is not implemented by RscriptRequirement
[   83s] __________________ ERROR collecting tests/r/RLintBearTest.py ___________________
[   83s] tests/r/RLintBearTest.py:1: in <module>
[   83s]     from bears.r.RLintBear import RLintBear
[   83s] bears/r/RLintBear.py:19: in <module>
[   83s]     prerequisite_check_fail_message='R library "lintr" is not installed.')
[   83s] bears/r/RLintBear.py:28: in RLintBear
[   83s]     version='>=1.0.2'),
[   83s] /usr/lib/python3.7/site-packages/dependency_management/requirements/RscriptRequirement.py:50: in __init__
[   83s]     'Setting version is not implemented by RscriptRequirement')
[   83s] E   NotImplementedError: Setting version is not implemented by RscriptRequirement

[frextrite]

As indicated by the traceback, this line is the cause of error:

if version:
    raise NotImplementedError(
        'Setting version is not implemented by JuliaRequirement')

If a version string is supplied the above exception is raised. This workflow was introduced in this issue. Thoughts?

[jayvdb]

We want the version in our metadata in bears repo, so we can describe the dependencies accurately, but unfortunately dependency_management doesnt know how to install specific versions for many types of requirements.

We also want dependency_management classes to indicate when they dont support versions.

iirc, this problem only appears when the dependency is missing.

It is annoying that it is happening in from bears.r.FormatRBear import FormatRBear.

We need the imports to succeed.

The quick fix is to remove the version from the RscriptRequirement inside the bears, but add DistributionRequirement with the version (this is deprecated; split into other classes), or an ExecutableRequirement with the version (and possibly implement a hacky version check inside ExecutableRequirement).

Then we can still get the version; this will require enhancing .ci/generate_bear_requirements.py

[frextrite]

Are these 2 the only bears throwing this error? In that case, we can remove the version string from both of them. Though we have other alternatives aswell:

1. implement the method to download the listed version(I believe this wasn't possible that's why a change was made to raise this exception)
2. undo the change which introduced this exception(not preferable though)
3. use dependency_management v0.4.0 which doesn't have this exception(0.4.0 was released around April 2018 and this exception change was introduced in July 2018). I believe the above log is from openSUSE package build. I ran through the logs and found that the dependency version which is being used is 0.5.x-dev. 0.4.0 is used in Travis and it works just fine.

Change 3 is the easiest one to implement if we are looking for a quick fix.

[jayvdb]

option 1, even if it is possible for Rscript, there will always be package managers which only support fetching/installing the current version. Go is another one where this is a problem, likewise Julia.

option 2, no, we need this.

option 3, no, that isnt possible/appropriate , see coala/meta#123

Please approach this like suggested in my last comment, which is the only path with moves us forward rather than backwards.

[frextrite]

@jayvdb Currently, the generate_bear_requirement.py gets the version number from each bears' .py file and then generates a curated list of requirements as bear-requirements.yaml.

In this issue, the NotImplementedError exception was implemented for Go and Julia apart from R. Since Go and Julia, both do not support installation of a specific package version, they do not have any entries in bear-requirements.yaml. Only 2 bears that do not support version installation but have an entry in bear-requirements.yaml are FormatRBear and RLintBear. Since they do not support version installation, their entry in the yaml file is redundant. It gives and I quote, "a false expectation that only that version would be installed".

So the best option would be to remove the version from bear file. Instead of this, we could add a distribution version(if there exists) but as you said it is now deprecated. And regarding changing ExecutableRequirement, the module currently does not support version checking. It just queries and returns if the executable is installed in the path.

[jayvdb]

Since they do not support version installation, their entry in the yaml file is redundant. It gives and I quote, "a false expectation that only that version would be installed".

Well the YAML doesnt give a false expectation -- it only describes the bears. Those entries are not redundant - they are the mechanism by which we will be able to create installers which can choose versions, or fail to install problematic versions.

Removing the version from the bear is not happening. We are not going backwards. Version metadata is incredibly important to be able to manage so many linters. It needs to be in the bear as the single source of truth, and extracted to yaml for other programs to easily obtain it. (we have an ansible installer on gitlab which uses this yaml, and the docker image will soon be updated to use it also)

Instead of this, we could add a distribution version(if there exists) but as you said it is now deprecated.

Moot point. It is deprecated but there are replacements which are not deprecated.

And regarding changing ExecutableRequirement, the module currently does not support version checking. It just queries and returns if the executable is installed in the path.

Again moot point. I suggested earlier that basic version checking be added to ExecutableRequirement.

[frextrite]

Well the YAML doesnt give a false expectation -- it only describes the bears.

I was referring to the version argument passed during instantiation of RscriptRequirement as stated by you.

PS: A detailed summary to follow up.

[frextrite]

Those entries are not redundant - they are the mechanism by which we will be able to create installers which can choose versions, or fail to install problematic versions.

In that case, we still need to have a version metadata somehow and the imports must not fail.

I dug further into PackageRequirements.py RscriptRequirement.py ExecutableRequirement.py and DistributionRequirement.py to solve the problem as per your guidelines, and here's what I found:

1. Basic version checking of a package cannot be implemented in ExecutableRequirement since it only checks for the existence of Executable in the path and has no way(neither can something be implemented) to check for a package of an executable. Eg: It can determine if R is installed but can't have any way to determine if an R package, say formatr is installed or not.
2. The above check is actually implemented in package specific files(say RscriptRequirement) which extends PackageRequirement to set the package and the executable. RscriptRequirement implements the method by PackageRequirement to check if any package of an executable is installed or not.
3. Which leads us to only 2 solutions,
   a. Remove version from ScriptRequirement and instead use the package from DistributionRequirement. This may work, but then we don't have an exact version number of the package for bear-requirements.yaml OR
   b. (the better solution)We remove version data from *ScriptRequirements declaration, instead, we add a metadata parameter which holds the version number(and optionally some other data). (metadata={version: '1.x.x'})This solves two things,
   • Since the R package cannot install a specific version of a package, we can skip the version parameter(version=''). However, the .yaml file can still be generated with the version information from metadata paramter.
   • We can use the existing method to check if the installed version complies with the version number provided in metadata parameter or not.
   • And more importantly it won't setup a false expectation that it is the current version that must be installed.

Let me know your thoughts.

[jayvdb]

Wrt to ExecutableRequirement, https://gitlab.com/coala/package_manager/issues/206 is a way to achieve that.

Almost all of the linters use a CLI. However not surprisingly, formatr and lintr have an executable of Rscript, and JuliaLint is another case.

Slightly related, wrt adding generic metadata, the method we intend to use is https://gitlab.com/coala/package_manager/issues/159

wrt DistributionRequirement, actually PlatformRequirement subclasses, using them does allow the bear to give exact versions. It is slightly annoying that the same version might appear multiple times due to using multiple PlatformRequirement subclasses where appropriate, but they refer to real versions obtainable by the user. A version metadata without any context of a distribution channel is a fictitious thing. e.g. the version of a package that you get from Debian may be incompatible with the same version obtained from RedHat. We encounter this all the time. e.g. verilator is still broken on openSUSE, but it was fixed on Redhat & Debian iirc. licensecheck is another one which is wildly different. And with R, the versions of linters needed are different depending on the version of R core - Haskell is similar.

[jayvdb]

Another approach is to add a requirement class for another R package manager. https://rstudio.github.io/packrat/walkthrough.html looks like it supports versions.

[jayvdb]

Created https://gitlab.com/coala/package_manager/issues/208 and https://gitlab.com/coala/package_manager/issues/209 as two additional ways this can be solved.