#105 CheckBundledJars could be automated
Closed: Fixed None Opened 11 years ago by mizdebsk.

This check says "If source tarball includes bundled jar/class files these need to be removed prior to building". If there are no .class or .jar files anywhere in any of sources, this test should pass.


This test should be performed not on the sources, but on the patched sources under BUILD (to take whatever done in %prep into account)

Related to #73; classes created during %build are present when checks run and thus reported if one tries to run a check.

This test should be performed not on the sources, but on the patched sources under BUILD (to take whatever done in %prep into account)

This would require having either available build directory (not available with --prebuilt) or installing all BuildRequires and running %prep in mock.

A much simpler solution that would work for the majority of packages would be checking upstream sources (if there are no bundled jars and classes then the check passes, otherwise it's inconclusive). My reasoning for this solution is that if there are bundled jars then the reviewer should check if they don't contain disallowed content (like non-free documentation, trademarks etc.) because in this case repacking upstream sources could be necessary.

I want to highlight that the great majority of all Java packages do not have bundled jars or classes, so my solution would automate the check in most cases, still being simple and runnable with --prebuilt.

Fixed in 6cb7878, using the patched sources. While I see your point about the specific java issues here, the issue of the state of the sources is an overall one, truly generic. So, these patches basically fixes so that the sources available to check (under BUILD), are as after rpmbuild -bp.

As for the prebuilt case, I'm not that convinced it should be fixed. Basically, using --prebuilt means that here is no build environment -> limitations. This certainly not the only one. But, of course, the java case is somewhat special here given the overall java practise to bundle jars (hmmm, isn't there something called "Ruby" around :) )

You might note that the fix includes a shell script which, by coincidence, is very similar to an example an in an email I read.

Thanks for the report. If you feel that a check for jars in the upstream sources still is needed, please feel free to reopen this bug (or file a new).

You might note that the fix includes a shell script which, by coincidence, is very similar to an example an in an email I read.

No' it's completely different. I don't like this solution for several reasons.

The most important one is that it doesn't work at all with --prebuilt (and as I mentioned before that's my main operation mode of F-R -- I want to review packages that are already in Fedora repository and have already been built in Koji).

Also it doesn't catch some other important things (like bundled non-free components) and has other limitations (directory name must start with %{name}, unquoted directory name, uses cd without leading ./ (what if CDPATH is set?), polutes review report with possibly hundreds or thousands of lines listing all class files.

Sorry, I didn't notice comment:5 until today.

Long story short your remarks are certainly reason to reopen this bug. Expanding on them I can see a number of things here:
- The need to create attachments from the script side. If you can provide an example of a package
with the problems of a too long listing, I think that would be a great starting point to implement
attachments. It's certainly in the cards, so to speak.
- The need to analyze the --prebuilt case i e., to use the sources in the srpm if the BUILD sources
are not available. This would be a rather straight-forward enhancement of this script.
- The need to look for bundled non-free components. Isn't this another check?
- The need to define the environment the checks run in. This should be set explicit in the python
code and documented. Or?
- The unquoted directory name is a plain bug which should be fixed.

At a second thought, the missing sources shouldn't be a problem. As of today (devel), the sources are unpacked using rpmbuild -bp --nodeps, which means that they should more or less 'always' be available (and that much of the wiki documentation is outdated). This is true also when using --prebuilt. Using -nodeps in this means that the deps are not installed or required.

One problem less?!

Fixed the unquoted directory name in a407eec. Still one problem less?!

I have pushed db61a2e. With this:
- There's a new function to attach data from the shell-script.
- The environment is cleared when the script is started. Although this blocks a way to handle
data to the script, I think the dependency on a random environment is worse. This solves the
'whatif CDPATH' issue, and potentially many more.
- As noted in comment:7, --prebuilt should work as well
- The (rather silly) dependency that sources have names starting with %{name}/$FR_NAME is removed.
- The output is stored in a file or as an attachment depending on size. Thus, there will be no
hundreds or thousands of lines of class files in the report.

This leaves search for non-free components open. This is another test IMHO.

Without more input, and with no clear idea how to improve this: closing. Thanks for the report, it has certainly helped to push things forward. Please feel free to reopen report if need be.

Login to comment on this ticket.

Metadata