bcgit / pc-dart

Pointy Castle - Dart Derived Bouncy Castle APIs
MIT License
237 stars 122 forks source link

Publish to/update for pub.dev #8

Closed AKushWarrior closed 4 years ago

AKushWarrior commented 4 years ago

https://pub.dev is the central repository for all Dart packages. Without pub, this repository cannot be used by dart code that desires to have control over versioning.

Currently, https://pub.dev/packages/pointycastle package still points to the old GitHub repository and the HKDF commit has not been uploaded.

AKushWarrior commented 4 years ago

Maybe @stevenroose needs to transfer ownership of PointyCastle on pub?

hoylen commented 4 years ago

+1

Without publishing a new release to pub.dev, all the links point to the old repository and users might not realise development is now being done under this new repository. I see people are still posting new issues on the old repository. The migration to the Legion of the Bouncy Castle won't be complete until it can publish a new version to pub.dev.

stevenroose commented 4 years ago

Sure. I'm open to that. I have been in e-mail communication with the BC team. If the same people that I've been in contact with contact me through the same way there (I don't know their GitHub handles so I can't verify their identity here), we can discuss pub ownership.

Initially I thought that for the first few new releases, they would prepare the release and I could publish it until I felt that they had taken over the project in a way that makes me comfortable handing over control entirely to them. I think that would still be a good idea.

So I'm totally open to be directed to publish a new release on pub. Either a dummy release to update the spec information or an actual software release.

hoylen commented 4 years ago

Yes, if you are still around and available, then a gradual transition with you doing the initial few releases makes sense.

A dummy release would be good too, since (from the outside) it appears the BC Team is taking time to get up to speed.

AKushWarrior commented 4 years ago

I think that sounds like a good idea.

@mwcw, you've been managing the repo (for the most part). Thoughts?

mwcw commented 4 years ago

Hi,

@stevenroose I will send you an email.

Just as a note: We are just getting a handle on how to manage this project, so any suggestions are most welcome.

AKushWarrior commented 4 years ago

@mwcw I'm thinking of writing a full guide on using this library. Would that help you?

mwcw commented 4 years ago

@mwcw I'm thinking of writing a full guide on using this library. Would that help you?

Yes please!

That would also help more people than just us.

Thank you for offering.

AKushWarrior commented 4 years ago

Okay :).

After I finish with ChaCha20-Poly1305, I'll get started with that.

AKushWarrior commented 4 years ago

@mwcw Starting the guide now. Regarding the original topic of this issue, my latest PR has that (with a lot of other stuff as well).

hoylen commented 4 years ago

Some introductory topics are already covered by the tutorials at the bottom of the README file: using Pointy Castle for digest and HMAC calculation, AES encryption/decryption, RSA signing/verification and encryption/decryption.

For your "full guide" it would be good if you can focus on the many other topics they do not yet cover. Also dartdoc documentation of the classes would be really useful too.

AKushWarrior commented 4 years ago

dartdocs would be good, but that's a LOT of docs I'd have to write.

AKushWarrior commented 4 years ago

RE tutorials: Maybe I'll link to them, or copy paste them over.

I really want people to have one site to go to when looking for resources. I know that it would have saved me a ton of time.

hoylen commented 4 years ago

Links are better. A copy can get out of date, requires more effort to maintain and can confuse people into thinking there are two different sets of tutorials (especially if they start to diverge).

What "one site" were you thinking of?

The first place people would look is the documentation linked from the package on pub.dev. Hence my desire for publishing a new release to pub.dev, so the tutorials can be easily found.

The second place people will look is the "API documentation" link from that pub.dev page. So that is why dartdoc documentation would be useful. Some dartdoc would be better than none!

Anyway, this discussion about documentation should be taken off this issue, since the topic of this issue is about publishing a new release to pub.dev.

AKushWarrior commented 4 years ago

Right. Maybe migrate this to the "write a guide" issue, or open a new "dartdocs" issue.

AKushWarrior commented 4 years ago

The one site would be a pointycastle.github.io link or something, which we put on top of the README. The site I created is kind of a pilot to see if the idea works.

AKushWarrior commented 4 years ago

@stevenroose @mwcw is there any progress on the original topic of this issue? The repository has now diverged pretty significantly from the current pub release.

AKushWarrior commented 4 years ago

This repository is prepped for publishing. @stevenroose what would you need to publish it?

stevenroose commented 4 years ago

I think I left a comment before. I'd like to get an e-mail frm the new maintainers here (we've been in contact before), for them to tag a new release and then I can review changes and publish. I'd like to do one or two more updates like this and if all goes well, I should feel ok to hand over the pub rights to the BC project entirely.

AKushWarrior commented 4 years ago

Cool. @mwcw said somewhere that BC would look at release next week, so hopefully that email + tag is in the pipeline.

mwcw commented 4 years ago

Hi @stevenroose

Had a quick chat to @dghbc would you have any objection to reviewing HEAD?

We tag it when you are satisfied.

~~There is one more sync in the pipeline (c6d3a847ea6877b59b548b137aa77ce754b9b547) and it doesn't involve any code changes.~~

Let me know what you think?

MW

mwcw commented 4 years ago

Hi,

There is another sync in the pipeline (52ef0aea5303466c015463a61d8b193994490d78) that fixes linter issues, that should occur within an hour of this post.

@stevenroose Let me know how you go.

MW

AKushWarrior commented 4 years ago

@stevenroose @mwcw I pulled the latest commit from GitHub and published it (https://pub.dev/packages/pc-steelcrypt). This is purely for testing purposes; I'll discontinue it after we publish to https://pub.dev/packages/pointycastle.

This will make it easier for Steven to download and play around with our new changes, as well.

stevenroose commented 4 years ago

Well it's easier to look at code changes. Is the commit https://github.com/bcgit/pc-dart/commit/52ef0aea5303466c015463a61d8b193994490d78?

mwcw commented 4 years ago

Well it's easier to look at code changes. Is the commit 52ef0ae?

Yes it is.

Thanks.

stevenroose commented 4 years ago

K so I skimmed over the changes. Since I can't go verify all new implementations, I mainly looked at changes existing implementation and primarily for any obvious wrongdoing.

I think I can ACK 52ef0aea5303466c015463a61d8b193994490d78 for release. Do you guys want to do a 2.0.0-rc1 release candidate of some kind, perhaps?

One thing I noticed was

-  sdk: ">=0.8.10+6 <3.0.0"
+  sdk: ">=2.2.0 <3.0.0"

Although I trust the project management in your hands, it feels a bit sad to see support for pre-v2 Dart be dropped. Even though I have no idea about usage distribution of different Dart versions.

The Dart SDK version bump means that we have to do a breaking version bump as well, though. So 2.0.0-rc1 seems like a good idea to me.

Ephenodrom commented 4 years ago

The update to version 2.2.0 or higher removes the following linter message :

Set literals weren't supported until version 2.2, but this code is required to be able to run on earlier versions.
Try updating the SDK constraints.

The message appears due to the fact that in the registry.dart file a Set is used. The code was added in August 2018.

https://github.com/bcgit/pc-dart/commit/af6c68bc5a18d6934def0e2964360fcb08b8a38d#diff-a06e362f42a8d7d66025b3643efd4b4f

A possible fix for that is to use the following code to still be able to use a lower dart sdk version :

Set<DynamicFactoryConfig>.of([]));

But this will result in the following linter message :

Use collection literals when possible.

So for my understanding, all people using at least version >=1.0.0 of Pointycastle have already dart version >=2.2.

AKushWarrior commented 4 years ago

The update to version 2.2.0 or higher removes the following linter message :

Set literals weren't supported until version 2.2, but this code is required to be able to run on earlier versions.
Try updating the SDK constraints.

The message appears due to the fact that in the registry.dart file a Set is used. The code was added in August 2018.

https://github.com/bcgit/pc-dart/commit/af6c68bc5a18d6934def0e2964360fcb08b8a38d#diff-a06e362f42a8d7d66025b3643efd4b4f

A possible fix for that is to use the following code to still be able to use a lower dart sdk version :

Set<DynamicFactoryConfig>.of([]));

But this will result in the following linter message :

Use collection literals when possible.

I think this is a case where using ignore to get rid of the lint is valid. Pre 2.0 Dart is a huge use case and we shouldn't get rid of it for such a small issue.

Ephenodrom commented 4 years ago

Will the current version run with dart version < 2.2 or will we have to use

Set<DynamicFactoryConfig>.of([]));

and ignore linter ?

If it will only run with the above code changes, for my understanding, all people using at least version >=1.0.0 of Pointycastle have already dart version >=2.2.

Ephenodrom commented 4 years ago

Another hint: As suggested in issue #25 the fixnum package has this entry in the latest changelog.

Update minimum SDK constraint to version 2.1.1.
mwcw commented 4 years ago

Hi,

Sorry about my silence.

Thanks @stevenroose for the review, much appreciated.

If I understand this thread correctly we have the following issues to weigh up:

  1. Shifting to 2.2.x specific code will silence a linter warning and I assume because the "quality" of an api has been gamified by pub.dev this will impact the score.

  2. Shifting to 2.2.x will cause breaking changes to the code base causing it to fail on earlier versions and there is a large user base pinned to the earlier versions.

  3. We could follow the lead of fixnum package make the minimum 2.1.1. Which I just tried this using,

    void _addDynamicFactoryConfig(DynamicFactoryConfig config) {
    Set factories = _dynamicFactories.putIfAbsent(
        config.type, () => Set<DynamicFactoryConfig>.of([])); //  <DynamicFactoryConfig>{});
    factories.add(config);
    }

    and all the tests passed.

I am not inclined to drop support for users pre 2.2.x and certainly not without warning but by the same token the culture of the underlying platform seems to push projects to stay close to the leading stable edge. The key thing I feel is to give advanced warning and doing that jump now is likely to surprise the community.

With this in mind, how do you all feel about doing the following?

  1. Silenced the linter warning on that Set constructor and use the pre 2.2.x syntax.
  2. Move to 2.0.0-rc1
  3. Advise the community that a move to 2.2.x dart version is will occur after say six months. I am assuming we have some enterprise users and they will probably appreciate the heads up.

Let me know what you all think? Especially around the time frame, is it a reasonable time?

If we agree sufficiently I'll change that constructor and get it tagged and the candidate can be released!

MW

Ephenodrom commented 4 years ago

I think this is a good plan.

AKushWarrior commented 4 years ago

6 months is plenty of refactoring time from Dart 1 --> Dart 2. An enterprise system should be able to do this given a few days of dedicated work.

As an aside, I highly doubt a significant contingent of Dart users, enterprise or otherwise, are using pre v2 Dart; the ecosystem is HEAVILY dependent on the features introduced in Dart 2. Flutter, to date the main driver of Dart usage, came out after and is dependent on Dart 2.

Regardless, I agree with Ephenodrom, this is a good plan.

mwcw commented 4 years ago

Ok I have pushed and added the tag v2.0.0-rc1 it should sync within an hour of this update. @stevenroose

I'll add a v2.0.0-rc1 Issue as a catch all for any discoveries when it is released.

If I have messed something up in the meantime please add it here!

Thanks for your help, everyone, it is much appreciated.

MW

Ephenodrom commented 4 years ago

@mwcw We should add the new repository url in the pubspec.yaml file. Currently the hompage field still points to the old repository.

mwcw commented 4 years ago

@mwcw We should add the new repository url in the pubspec.yaml file. Currently the hompage field still points to the old repository.

Thanks

Fixed and retagged, it should sync up shortly as 7c8a8b47

MW

Ephenodrom commented 4 years ago

Any plans on adding my open PR #36 to the current release candidate? This would cut down the linter warnings to 500. So a good start to fix all more complicated linter warnings.

mwcw commented 4 years ago

Any plans on adding my open PR #36 to the current release candidate? This would cut down the linter warnings to 500. So a good start to fix all more complicated linter warnings.

Hi

I was going to wait until after the release went out but yes given how releasing works I will merge it in.

MW

penhorwood commented 4 years ago

Any ETA when this release will show up on https://pub.dev/packages/pointycastle?

My code needs a few of the bugs fixed in this next release to work correctly. It would be much easier to work with if pub.dev had the latest version.

AKushWarrior commented 4 years ago

Any ETA when this release will show up on https://pub.dev/packages/pointycastle?

I my code needs a few of the bug fixed in this next release to work correctly. It would be much easier to work with if pub.dev had the latest version.

@penhorwood

What bugs do you need fixed (what is the earliest working commit)? You may be able to use https://pub.dev/packages/pc-steelcrypt, which is a temporary publish I made for the interim until the official release. That pulls from an earlier version of this repository, but has most of the major new edits aside from some lint fixes and PSSSigner.

@stevenroose @mwcw hate to ping you guys again but is there a roadblock to release?

mwcw commented 4 years ago

Hi, I don't have any blockers.

@stevenroose MW

Ephenodrom commented 4 years ago

@mwcw and @stevenroose Another reason to publish the newest version to pub.dev. It will fix the problem mentioned in #40. I tried the same thing the user described in the issue with version 1.0.2 and faced the same issue. I updated to the current release candidate on github and it worked well. So with the latest version on pub.dev the RSA encryption/decryption is buggy and not ready to use in any production application.

mwcw commented 4 years ago

Hi,

@stevenroose how would you feel about beginning the process of delegating the ability to publish to the project?

MW

penhorwood commented 4 years ago

I'm still hoping this will make it over to pub.dev. It is not super complicated to add an external repo as a dependence in projects. I figured it out as a workaround but pub.dev appears to be the place for things dart related.

@stevenroose

hpoul commented 4 years ago

How about just publishing this under the bouncycastle package name, if that is still available.. shouldn't be too much work for users of the package, and maybe serve as a reminder of the change in ownership, and would be possible immediately ;-)

AKushWarrior commented 4 years ago

How about just publishing this under the bouncycastle package name, if that is still available.. shouldn't be too much work for users of the package, and maybe serve as a reminder of the change in ownership, and would be possible immediately ;-)

It is available. What do you think @mwcw? From my testing package, just having https://pub.dev/packages/steel_crypt as a downstream dependency gets the popularity rating to ~85%. Our "pub ratings" (god I hate that arbitrary gameification) will thus be fine even if we use a new package name.

@hpoul The main worry is that the entire user base is now going to have to depend on a different package. I guess Reddit/Medium posts could help alleviate some of that pain.

Ephenodrom commented 4 years ago

+1 for the idea to rename it to "Bouncycastle". Then every bouncycastle package ( java, c#, kotlin and dart ) will have the same name.

mwcw commented 4 years ago

Hi,

I had a very brief chat to @dghbc about it.

I understand that right now this will be unpopular but to rename and redeploy the project under a new package has the risk of being unnecessarily uncouth given we don't know what is going on, and given we are in the middle of pandemic anything could be happening.

However once we move past this, the project can move along, and everyone's contributions be formally released and deservedly so.

In the meantime I will have more formal discussion internally about what we need to set up at our end to be able to deploy.

MW

penhorwood commented 4 years ago

I believe @stevenroose should be given credit for all of the work he put into pointy castle. I also think changing the packaging to bouncy castle makes the most sense due to the other bouncy castle packages. Bouncy Castle has a great reputation in the Java world.

AKushWarrior commented 4 years ago

I believe @stevenroose should be given credit for all of the work he put into pointy castle. I also think changing the packaging to bouncy castle makes the most sense due to the other bouncy castle packages. Bouncy Castle has a great reputation in the Java world.

This is very true: BouncyCastle has cache as a name and brand, and a good number of Dart developers are also familiar with Java.