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

Add library for publishing artifacts to crates.io #194

Merged
merged 2 commits into from
Apr 25, 2023

Conversation

gaiksaya
Copy link
Member

@gaiksaya gaiksaya commented Apr 19, 2023

Description

Adds a library to publish rust artifacts to crates.io. The artifacts do not needs to be built similar to NPM. Also signing is not supported yet.

Issues Resolved

resolves #155
related opensearch-project/opensearch-build#3247

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@gaiksaya gaiksaya changed the title Add library for publishing arifacts to crates.io Add library for publishing artifacts to crates.io Apr 19, 2023
}

void parameterCheck(String repository, String tag) {
if (!repository || !tag) {
Copy link
Member

@peterzhuamazon peterzhuamazon Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be if (!repository && !tag)?

In the current implementation with ||, if there is NO repository but there IS a tag it will still error out.

Hopefully my jetlag mind is not messing this up. Thanks.

Copy link
Member Author

@gaiksaya gaiksaya Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes if either is not provided it should error out. We are checking the ! so || works here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No you didnt get me there.
if there is NO repository but there IS a tag

with if (!repository || !tag), if there is NO repo, then the 1st condition is true, and it will never check the 2nd condition. Therefore, if there is also a tag, which makes the 2nd condition false, will never be tested and it will default to error out despite you have a tag but no repository.

Copy link
Member Author

@gaiksaya gaiksaya Apr 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah so I want to error out if there is a tag but no repository which is correct.
I believe what you are saying is if (repository && tag) which is equal to if (!repository || !tag) that I have above. So if either of them (or both) is not present it should throw an error.
I tested this. Works as expected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have offline discussion with Sayali now, understand her intention is to test if tag exist when there IS a repository. If there is NO repository, then dont even need to care.

Suggest adding some comments on this in case the context is missed later, or just use 2 nested if condition for more clarity.

Thanks.

checkout([$class: 'GitSCM', userRemoteConfigs: [[url: "${args.repository}" ]], branches: [[name: "${args.tag}" ]]])

withCredentials([string(credentialsId: 'crates-api-token', variable: 'API_TOKEN')]) {
sh "cargo publish ${packageToPublish} --dry-run && cargo publish ${packageToPublish} --token ${API_TOKEN}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if a package is not passed?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just out of curiosity to what exactly gets published. Approving.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, the package in the current working directory is selected if package is not passed. Usually if there are more packages, cargo errors out asking to select a package exclusively.

Signed-off-by: Sayali Gaikawad <[email protected]>
@gaiksaya gaiksaya merged commit c8033da into opensearch-project:main Apr 25, 2023
@gaiksaya gaiksaya deleted the rust branch April 25, 2023 18:41
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 25, 2023
Signed-off-by: Sayali Gaikawad <[email protected]>
(cherry picked from commit c8033da)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gaiksaya pushed a commit that referenced this pull request Apr 25, 2023
(cherry picked from commit c8033da)

Signed-off-by: Sayali Gaikawad <[email protected]>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

publishToCrates - publish to crates.io (rust artifacts)
3 participants