Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#40068 closed enhancement (fixed)

subversion: change dependency on curl-ca-bundle to support certsync

Reported by: neverpanic (Clemens Lang) Owned by: blair (Blair Zajac)
Priority: Normal Milestone:
Component: ports Version: 2.2.0
Keywords: Cc: danielluke (Daniel J. Luke), cooljeanius (Eric Gallager)
Port: subversion

Description

Please modify the dependency on curl-ca-bundle in subversion to support the new certsync port (see #35474 for discussion). I'm attaching a patch to do this.

Attachments (1)

subversion-certsync.patch (392 bytes) - added by neverpanic (Clemens Lang) 11 years ago.

Download all attachments as: .zip

Change History (15)

Changed 11 years ago by neverpanic (Clemens Lang)

Attachment: subversion-certsync.patch added

comment:1 Changed 11 years ago by cooljeanius (Eric Gallager)

Personally, I'd keep the default to path:share/curl/curl-ca-bundle.crt:curl-ca-bundle as you have it now, but change it to path:share/curl/curl-ca-bundle.crt:certsync when the OS X keychain variant (+osxkeychain) is enabled (which is the default for platform macosx as of r109123). (although currently the official port still uses +disable_keychain, so for the trunk version it'd have to be switched)

Last edited 11 years ago by cooljeanius (Eric Gallager) (previous) (diff)

comment:2 Changed 11 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:3 in reply to:  1 ; Changed 11 years ago by larryv (Lawrence Velázquez)

Replying to egall@…:

Personally, I'd keep the default to path:share/curl/curl-ca-bundle.crt:curl-ca-bundle as you have it now, but change it to path:share/curl/curl-ca-bundle.crt:certsync when the OS X keychain variant is enabled.

Uh, why? Users will already have the option to satisfy the dependency by manually installing curl-ca-bundle; there’s no reason to complicate the Portfile further.

comment:4 in reply to:  3 ; Changed 11 years ago by cooljeanius (Eric Gallager)

Replying to larryv@…:

Replying to egall@…:

Personally, I'd keep the default to path:share/curl/curl-ca-bundle.crt:curl-ca-bundle as you have it now, but change it to path:share/curl/curl-ca-bundle.crt:certsync when the OS X keychain variant is enabled.

Uh, why? Users will already have the option to satisfy the dependency by manually installing curl-ca-bundle; there’s no reason to complicate the Portfile further.

certsync provides integration with the OS X keychain, if a user specifies that they want OS X keychain integration by selecting the relevant variant (+osxkeychain for larryv's branch (which is the default for platform macosx), or -disable_keychain for trunk), and has neither certsync nor curl-ca-bundle installed yet, I would expect that MacPorts would pull in the port that better integrates with the keychain (i.e. certsync) to satisfy the depspec instead of the default curl-ca-bundle. Conversely, if the user disables the keychain variant (-osxkeychain for larryv's branch, or +disable_keychain for trunk), that shows that the user is actively trying to avoid keychain-related stuff (perhaps, because they're on a platform other than macosx that doesn't use the keychain), so it would make sense that they would want to avoid certsync by having curl-ca-bundle installed for them instead.

Last edited 11 years ago by cooljeanius (Eric Gallager) (previous) (diff)

comment:5 in reply to:  4 ; Changed 11 years ago by larryv (Lawrence Velázquez)

Replying to egall@…:

certsync provides integration with the OS X keychain, if a user specifies that they want OS X keychain integration by selecting the relevant variant, and has neither certsync nor curl-ca-bundle installed yet, I would expect that MacPorts would pull in the port that better integrates with the keychain (i.e. certsync) to satisfy the depspec instead of the default curl-ca-bundle.

I’m saying that certsync should always be automatically installed to satisfy that dependency, if necessary. Users should not be given an option, other than the implicit one of manually installing curl-ca-bundle beforehand.

comment:6 Changed 11 years ago by danielluke (Daniel J. Luke)

Yeah, I agree, we should just use certsync and not curl-ca-bundle for this.

comment:7 Changed 11 years ago by danielluke (Daniel J. Luke)

Resolution: fixed
Status: newclosed

comment:8 in reply to:  5 ; Changed 11 years ago by cooljeanius (Eric Gallager)

Replying to larryv@…:

Replying to egall@…:

certsync provides integration with the OS X keychain, if a user specifies that they want OS X keychain integration by selecting the relevant variant, and has neither certsync nor curl-ca-bundle installed yet, I would expect that MacPorts would pull in the port that better integrates with the keychain (i.e. certsync) to satisfy the depspec instead of the default curl-ca-bundle.

I’m saying that certsync should always be automatically installed to satisfy that dependency, if necessary.

By that logic, you shouldn't have bothered wrapping the +osxkeychain variant in a platform macosx block in r109123.

Users should not be given an option

Why not? As a user, I would always want the option.

comment:9 in reply to:  8 Changed 11 years ago by danielluke (Daniel J. Luke)

Replying to egall@…:

I’m saying that certsync should always be automatically installed to satisfy that dependency, if necessary.

By that logic, you shouldn't have bothered wrapping the +osxkeychain variant in a platform macosx block in r109123.

this isn't the place to discuss this (the mailing list would be more appropriate) - but you should note that that revision is for something in /branches and not the official port (which still has a negative variant).

Users should not be given an option

Why not? As a user, I would always want the option.

part of what makes using macports good is that maintainers make a lot of decisions for you to give you something that just does what you want (most of the time). If your needs aren't met by the port, it's often an indication that you shouldn't be using the port (and just build it yourself) rather than something that the port is lacking.

... again, please take additional discussion to the mailing list(s).

comment:10 Changed 11 years ago by larryv (Lawrence Velázquez)

Since we probably don’t want new users to get a certsync port that doesn’t autoload, I conditionalized the dependency in r109305. We can remove it whenever startupitem.autostart makes it into a base release—probably the next one.

comment:11 Changed 11 years ago by danielluke (Daniel J. Luke)

Since the certsync launchd plist only needs to be loaded to keep the certs sync'd (on install, it gives you a working $prefix/etc/openssl/cert.pem in postactivate). I don't think the changge in r109305 is necessary.

I'm not going to remove it, though.

In the future, please at least try to contact me before committing any changes to my non-openmaintainer ports.

comment:12 in reply to:  11 Changed 11 years ago by larryv (Lawrence Velázquez)

Replying to dluke@…:

Since the certsync launchd plist only needs to be loaded to keep the certs sync'd (on install, it gives you a working $prefix/etc/openssl/cert.pem in postactivate). I don't think the change in r109305 is necessary.

You’re right, I think. Even if a user installs certsync with 2.2.0 and never manually loads the plist, it looks like it would get loaded the first time they upgrade certsync after installing a version of base that does support autoloading. So it would work out, eventually.

In the future, please at least try to contact me before committing any changes to my non-openmaintainer ports.

My fault, wasn’t thinking. Won’t happen again. Reverted in r109311.

comment:13 Changed 11 years ago by jmroot (Joshua Root)

Did you notice that the builds kicked off after r109302 timed out while activating certsync? Probably ought to look into that.

comment:14 in reply to:  13 Changed 11 years ago by danielluke (Daniel J. Luke)

Replying to jmr@…:

Did you notice that the builds kicked off after r109302 timed out while activating certsync? Probably ought to look into that.

I would guess update-ca-certificates fails on the buildbot, then maybe it can't connect to the keychain for some reason? That's probably something that there should be a new ticket for ;-)

Note: See TracTickets for help on using tickets.