101companies / 101worker

4 stars 11 forks source link

Haskell fact extractor to include startLine and endLine keys #198

Open rlaemmel opened 9 years ago

rlaemmel commented 9 years ago

cc @martinleinberger

Currently these line ranges are coming from the obsolete fragment locator.

Once done, Martin would switch to the improved fact extractor.

rlaemmel commented 9 years ago

Done: https://github.com/101companies/101worker/commit/25f59db7f3e334c7821dc03f9508790ccb109fcb Also removed the obsolete fragment locator: https://github.com/101companies/101repo/commit/1f1315144a6d691f1e98042c06e04eee1d0163ae

BTW, I also removed executables / object code for fact extractor from repo, which has been added some time ago.

@fruether, perhaps check that this was not also done for other plugins. Use of .gitignore is advised.

fruether commented 9 years ago

We droped and will in the future ignore the ".dll", ".exe" and ".class" files in the bin/ folder of the extractors. @kevin-klein: That perhaps has a side-effect! The worker perhaps has to build those removed executables again. That could be achieved by running the /install in the extractors directory

hartenfels commented 9 years ago

@fruether @kevin-klein that might be a bad idea, since install is run as root. If that kind of building is necessary, then we need to introduce a build script or something that runs Makefile targets as non-root. If y'all think it's a good idea, then I'll have it set up in like 5 minutes.

fruether commented 9 years ago

If we want to avoid uploading unnecessary binaries then we have to build those files before the extarctors get executed by the runner (follow extractors are affected: CSharp, Java,). If install isn't a good idea, and we neither can use test since it has another purpose, the build script seems to be the best idea to me.

kevin-klein commented 9 years ago

i think that install is a bad idea because we intended install to work as a one-time setup. I think we should just create a module that builds them every night after we update all repos on the worker.

fruether commented 9 years ago

There is no need for another module. The extractor module could itself build those files if there was a change by running a "build" script in the extractors folder

hartenfels commented 9 years ago

@kevin-klein Well, how about we just stick the thing into a module then, instead of a script? The principle is still the same.

@fruether Yes, but we need something that runs those scripts automatically.

fruether commented 9 years ago

@hartenfels if we modify the extractor101 module to call that script if there was a change in the extractor/ directory we would have a module that runs those automatially. At least when the worker runs. But perhaps it would be convenient to release both options.

  1. A script that runs after installation without root privs to make sure everything is fine after the setup
  2. Let the extract101 module compile the files if there was a change after the first setup e.g. when the Jfactextractor was modified.
hartenfels commented 9 years ago

@fruether alright, I get it now. That'd work if the extractors are the only things that need to be built, but I think there might be others too. In that case, it'd be easier to just have one thing that builds them all.

fruether commented 9 years ago

The validator scripts build themself if required: https://github.com/101companies/101worker/blob/master/validators/CSharp/validator

That would be another option. The predicates are plain python so far so no need for those to be compiled

Edit: the disadvantage with letting the script build their parts themself, like the validatos do, is that if for some reason the developer forgets to do that the worker will crash in production.

kevin-klein commented 9 years ago

I meant "a module" as a conceptual idea to make "some module" ensure that all extractors are built before they are intended to be used by some other module. Here we have to keep in mind that the explorer needs them and there have to be up-to-date binary compilations of them after every nightly update. @fruether I think the failing part is just a normal issue and we can't do anything about it.

hartenfels commented 9 years ago

@kevin-klein see #201 for a concept that would do exactly this.