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

CheckstyleBear: use binary checkstyle if it exists #1250

Open
jayvdb opened this issue Jan 3, 2017 · 12 comments
Open

CheckstyleBear: use binary checkstyle if it exists #1250

jayvdb opened this issue Jan 3, 2017 · 12 comments

Comments

@jayvdb
Copy link
Member

jayvdb commented Jan 3, 2017

The bear is currently downloading http://sourceforge.net/projects/checkstyle/files/checkstyle/6.15/checkstyle-6.15-all.jar , even if checkstyle is installed in the system.

Fedora 25 packages checkstyle 7.1.1, and includes a bash script checkstyle
Ubuntu xenial+ has 6.15, and includes a bash script checkstyle. trusty as 5.6.
openSUSE has 4.3 and 5.8 available

There is a slight problem with versioning, as the bear provides five preset rulesets. The sun and google rulesets are packaged with checkstyle since at least version 6.2, so the syntax of the ruleset will be correct for the installed version. In addition, Ubuntu trusty package for checkstyle 5.6 and precise does include those rulesets.

The other three rulesets are xml files on the internet, and havent been tests against different versions of checkstyle. And when the ruleset uses features not in the installed version of checkstyle, checkstyle fails badly (see #1034).

@Techievena
Copy link
Member

@jayvdb Can I work on this issue?

@Techievena
Copy link
Member

I think solving #1251 will be the first step for it?

@Techievena
Copy link
Member

This is a binary version of checkstyle https://sourceforge.net/projects/checkstyle/files/checkstyle/6.15/checkstyle-6.15-bin.zip. But shouldn't we now use MavenRequirement from dependency management to get checkstyle. MavenRequirement will completely solve this issue

The bear is currently downloading http://sourceforge.net/projects/checkstyle/files/checkstyle/6.15/checkstyle-6.15-all.jar , even if checkstyle is installed in the system.

@Techievena
Copy link
Member

I don't get it how using a binary version will sort out this problem. Any help on this?

@jayvdb
Copy link
Member Author

jayvdb commented Jan 21, 2017

Once we upgrade dependency_management (Im working on it), we can add a MavenRequirement, but keep in mind that there is no voodoo - it is only metadata. The code currently fetches checkstyle, and that needs to be deprecated.

@Techievena
Copy link
Member

Can't we prevent that using is_installed( ) in Maven Requirement? @jayvdb

@jayvdb
Copy link
Member Author

jayvdb commented Jan 21, 2017

coala/coala#3605 is merged, so you can now use MavenRequirement. Ya, the current download code can be skipped if it is already installed, but then you need to find the .jar file as it contains the .xml rulesets, so this issue could be more interesting. Have fun. ;-)

@gaocegege
Copy link
Member

gaocegege commented Feb 5, 2017

Hi, I meet a problem when I'm working on #1366

I have no idea how to get the local maven repository path. If we want to run the jar file we need to know the path to it, and I think different OSes have different paths:)

@Techievena
Copy link
Member

@gaocegege I was working on some other issues and will resume my work on this from now. Well I there are two possible ways we can fix this imo 😄
1)Cache the jar files by some method
2)Change M2_HOME M2 and other path variables for maven to download jar files in path we wish for
__P.S.:There is an option -DoutputDirectory for mvn installation maybe we can make suitable changes in MavenRequirement. I am currently seeing how to use that option correctly. __ 😄

@gaocegege
Copy link
Member

gaocegege commented Feb 5, 2017

@Techievena I think -DoutputDirectory won't reuse the dependency in local maven repo. 🤔

For example, if the checkstyle have been installed in ~/.m2, and we specify a directory to install it by mvn, it doesn't solve this issue.

@Techievena
Copy link
Member

Path to checkstyle.jar --> os.path.join(os.environ['HOME'],'.m2','repository','com','puppycrawl','tools','checkstyle','6.15') . May help in solving
#1366

@jayvdb jayvdb modified the milestones: 0.11, 0.12 Apr 25, 2017
@Techievena
Copy link
Member

@jayvdb Isn't this similiar to #1653 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants