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 publishToNuget lib and fix workspace overwrite issue for signing #117

Merged
merged 6 commits into from
Feb 3, 2023

Conversation

gaiksaya
Copy link
Member

Signed-off-by: Sayali Gaikawad gaiksaya@amazon.com

Description

Issues Resolved

resolves opensearch-project/opensearch-build#1436
resolves opensearch-project/opensearch-build#2965

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 requested a review from a team as a code owner January 28, 2023 04:52
@codecov-commenter
Copy link

codecov-commenter commented Jan 28, 2023

Codecov Report

Merging #117 (9ec7e9d) into main (ea7a7cc) will increase coverage by 0.08%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##               main     #117      +/-   ##
============================================
+ Coverage     83.41%   83.50%   +0.08%     
  Complexity       24       24              
============================================
  Files            68       69       +1     
  Lines           193      194       +1     
  Branches         11       11              
============================================
+ Hits            161      162       +1     
  Misses           25       25              
  Partials          7        7              
Impacted Files Coverage Δ
tests/jenkins/jobs/PublishToNuget_Jenkinsfile 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@prudhvigodithi
Copy link
Member

@gaiksaya LGTM, only code we have to look for is for package in find src -name OpenSearch*.nupkg``, there is no chance the package that we publish will be without OpenSearch prefix as we allow to publish only with `OpenSearch` prefix to the dotnet account, but I'm just thinking if any dependent packages exists without `OpenSearch` prefix, if so we should work with dotnet repo team.

@gaiksaya
Copy link
Member Author

gaiksaya commented Feb 3, 2023

@gaiksaya LGTM, only code we have to look for is for package in find src -name OpenSearch*.nupkg``, there is no chance the package that we publish will be without OpenSearch prefix as we allow to publish only with `OpenSearch` prefix to the dotnet account, but I'm just thinking if any dependent packages exists without `OpenSearch` prefix, if so we should work with dotnet repo team.

Even if there are I believe we should not be signing them as our own. @Xtansia can confirm though is there is any possibility in future.

@prudhvigodithi
Copy link
Member

prudhvigodithi commented Feb 3, 2023

Even if there are I believe we should not be signing them as our own. @Xtansia can confirm though is there is any possibility in future.

Since those dependent package dll's will be inside the .nukpg file, its better we sign them as well as they are indirectly used by the main package

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
@Xtansia
Copy link

Xtansia commented Feb 3, 2023

Even if there are I believe we should not be signing them as our own. @Xtansia can confirm though is there is any possibility in future.

Since those dependent package dll's will be inside the .nukpg file, its better we sign them as well as they are indirectly used by the main package

The NuGet package only contains the DLL for the specific library, it doesn't contain dependencies DLLs, it's purely managed via metadata.
For example, the OpenSearch.Client library depends on the OpenSearch.Net library (which has it's own dependencies), but the OpenSearch.Client.x.y.z.nupkg only contains the OpenSearch.Client.dll (for each framework), and the OpenSearch.Net.x.y.z.nupkg only contains the OpenSearch.Net.dll (for each framework).

This can be seen here: https://nuget.info/packages/OpenSearch.Client/1.2.0

@Xtansia
Copy link

Xtansia commented Feb 3, 2023

And yes, all publicly consumable packages from opensearch-net do and will have the OpenSearch. namespace.

@gaiksaya
Copy link
Member Author

gaiksaya commented Feb 3, 2023

And yes, all publicly consumable packages from opensearch-net do and will have the OpenSearch. namespace.

Thanks! That answers our question.

@gaiksaya gaiksaya merged commit 8422ac8 into opensearch-project:main Feb 3, 2023
@gaiksaya gaiksaya deleted the publishToNuget branch February 3, 2023 01:44
opensearch-trigger-bot bot pushed a commit that referenced this pull request Feb 6, 2023
…117)

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
(cherry picked from commit 8422ac8)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
gaiksaya pushed a commit that referenced this pull request Feb 6, 2023
…117) (#131)

(cherry picked from commit 8422ac8)

Signed-off-by: Sayali Gaikawad <gaiksaya@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
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
4 participants