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

Depends field should be left empty if a package depends on no other #208

Closed
rspieldenner opened this issue May 7, 2015 · 12 comments · Fixed by #288
Closed

Depends field should be left empty if a package depends on no other #208

rspieldenner opened this issue May 7, 2015 · 12 comments · Fixed by #288
Milestone

Comments

@rspieldenner
Copy link
Contributor

the upgrade of Jdeb to 1.4 adds a default dependency of "default-jre | java6-runtime"

Not all packages need to depend on other packages. Also the debian control file syntax allows the Depends field to be omitted.

rspieldenner added a commit to rspieldenner/jdeb that referenced this issue May 7, 2015
@tmortagne
Copy link
Collaborator

It's not really that simple. The idea behind this automatic depends is that jdeb has been designed for java projects (and is mostly used for java projects). You don't have any dependency on java in a Maven project for example but you still need Java to work if you are building a package for a jar.

Currently (since #190) when you don't want any dependency you can explicitly set an empty Depends.

@tcurdt
Copy link
Owner

tcurdt commented May 29, 2015

I am also leaning towards not having a default anymore and adding it to all examples explicitly.

...but I have two concerns:

"default-jre | java6-runtime" is no longer a good default (given that we are at java8 now). Which begs the question what the default runtime should be? Just use java8? Changing it to java8 could almost as bad as just switching it to . The question is whether such a thing would be semantically OK for a 1.5 release.

Calling a release 2.0 just for that does not feel warranted - and this would change in 2.x anyway. (see #195)

@tcurdt
Copy link
Owner

tcurdt commented May 29, 2015

Since there is a work around to set it explicitly to empty I will close this issue for - but be sure it's kept in mind for 2.x. Thanks for reporting.

@tcurdt tcurdt closed this as completed May 29, 2015
@tcurdt tcurdt added this to the 2.0 milestone May 29, 2015
@ebourg
Copy link
Collaborator

ebourg commented May 29, 2015

The default version for the java-runtime dependency could probably be derived from the target level used by the compiler plugin. But java6-runtime isn't that bad since this dependency is also satisfied by the Java 8 packages.

@tcurdt
Copy link
Owner

tcurdt commented May 29, 2015

...and for ant?

Ah! Didn't know that java6-runtime is satisfied by java8 as well. That does make things a little easier if we want to change the default.

@ebourg
Copy link
Collaborator

ebourg commented May 29, 2015

For Ant we could default to the version of the JDK used to run jdeb.

@tcurdt
Copy link
Owner

tcurdt commented May 29, 2015

Meh. I'd say just leave it for the 1.x branch.
And for the 2.x version it should be more obvious.

@rspieldenner
Copy link
Contributor Author

This isn't absolutely true. Certain dependencies like asm are highly
sensitive to JRE version (3.x won't work in JRE 8, and 5.x doesn't work in
JRE 6).

On Fri, May 29, 2015 at 1:31 AM, Torsten Curdt [email protected]
wrote:

Meh. I'd say just leave it for the 1.x branch.
And for the 2.x version it should be more obvious.


Reply to this email directly or view it on GitHub
#208 (comment).

@krushik
Copy link

krushik commented Jun 15, 2015

The default list of possible dependencies (if reintroduced) should probably include 'java-runtime-headless', and/or 'default-jre' may be changed to 'default-jre-headless' as the thinnest requirement:

$ apt-cache show openjdk-7-jre-headless | grep Provides
Provides: java-runtime-headless, java2-runtime-headless, java5-runtime-headless, java6-runtime-headless, java7-runtime-headless
$ apt-cache show default-jre | grep Depends
Depends: default-jre-headless, openjdk-7-jre
$ apt-cache show default-jre-headless | grep Depends
Depends: openjdk-7-jre-headless (>= 7~u3-2.1.1)

@ebourg
Copy link
Collaborator

ebourg commented Jun 15, 2015

For now java-runtime-headless stands for Java 1, not really a good choice :) Also on some architectures default-jre doesn't pull OpenJDK but GCJ which is roughly equivalent to Java 5.

philwills pushed a commit to guardian/status-app that referenced this issue Jun 21, 2015
Otherwise JDeb does nonsense: tcurdt/jdeb#208
philwills pushed a commit to guardian/status-app that referenced this issue Oct 8, 2015
Otherwise JDeb does nonsense: tcurdt/jdeb#208
@tcurdt
Copy link
Owner

tcurdt commented Jan 16, 2016

I think we screwed ourselves into a corner here. I would really love to rid of the default Depends. Maybe we should bite the bullet and just print out a warning if there is no Depends. Adding the one line to the control file isn't that big a deal.

Thoughts? @ebourg ?

@tcurdt tcurdt modified the milestones: 1.5, 2.0 Jan 23, 2016
@tcurdt
Copy link
Owner

tcurdt commented Jan 23, 2016

Maybe let's get it into the 1.5 release and be done with it.

tcurdt added a commit that referenced this issue Jan 23, 2016
Omit Depends: from control file if not set. Fixes #208
jacobwil added a commit to enveil/jdeb that referenced this issue May 28, 2019
…s set.

When the change to no longer include the default depends was made in tcurdt#209 to fix tcurdt#208 it entirely eliminated the ability to set the depends programatically. In fact, the field DebMaker.depends is never read.
tcurdt pushed a commit that referenced this issue May 29, 2019
* Do include the depends field in the control file if and only if it was set.

When the change to no longer include the default depends was made in #209 to fix #208 it entirely eliminated the ability to set the depends programatically. In fact, the field DebMaker.depends is never read.

* Add unit test for ensuring depends is in control.

Also augment DebMakerTestCase#testDependsIsOmittedWhenEmpty to fail if control is missing entirely.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants