brandedoutcast / publish-nuget

📦 GitHub action to automate publishing NuGet packages when project version changes
https://github.com/marketplace/actions/publish-nuget
MIT License
197 stars 101 forks source link

[BUG] Sequential build and publish deleting nuget packages #56

Open sergeyshaykhullin opened 3 years ago

sergeyshaykhullin commented 3 years ago

Describe the bug Each build and push step deletes all nuget packages from previous one

Failed Action Log URL (Required) https://github.com/appany/AppAny.Quartz.EntityFrameworkCore.Migrations/runs/1913414360?check_suite_focus=true

To Reproduce

  1. Build and publish first package
  2. Build and publish second package
  3. Create release
  4. Try to upload first .nuget to artifacts
  5. BOOM

Expected Behavior Each build step has its own directory for .nuget files and do not delete previous

The workaround is to reorder steps

Here is the problem https://github.com/brandedoutcast/publish-nuget/blob/c12b8546b67672ee38ac87bea491ac94a587f7cc/index.js#L58

Environment:

AraHaan commented 3 years ago

@sergeyshaykhullin those logs expired.

AraHaan commented 3 years ago

I think a simple fix for this is to remove .forEach(fn => fs.unlinkSync(fn)) since usually people add nupkg's and snupkg's to their .gitignore. If they did not that is their fault if they use an action that commits them to the repository. Wait no I will just change that line to depend on the projectFile variable that it points to on that path.

sergeyshaykhullin commented 3 years ago

No, because if you will not delete previous nuget files you will try to push them again

https://github.com/brandedoutcast/publish-nuget/blob/c12b8546b67672ee38ac87bea491ac94a587f7cc/index.js#L67

AraHaan commented 3 years ago

So basically I guess one way to fix that is to nuke them after publish.

sergeyshaykhullin commented 3 years ago

@AraHaan No, i suggest to

Each build step has its own directory for .nuget files and do not delete previous

Randomly generate directories foreach GitHub Action run

So if you have:

- uses: brandedoutcast/publish-nuget@v2
  ...

- uses: brandedoutcast/publish-nuget@v2
  ...

You will have:

.../random/path/1/*.nuget

.../random/path/2/*.nuget

It means that:

  1. _pushPackage should generate random directory
  2. -o { directory } https://github.com/brandedoutcast/publish-nuget/blob/c12b8546b67672ee38ac87bea491ac94a587f7cc/index.js#L62
  3. push { directory }/*.nuget https://github.com/brandedoutcast/publish-nuget/blob/c12b8546b67672ee38ac87bea491ac94a587f7cc/index.js#L67
  4. readDirSync('{ directory }') https://github.com/brandedoutcast/publish-nuget/blob/c12b8546b67672ee38ac87bea491ac94a587f7cc/index.js#L64
  5. Something what i missed
sergeyshaykhullin commented 3 years ago

Also, as i know, this repo is abandoned

@drusellers What do you think about this changes?

AraHaan commented 3 years ago

I got a better idea and updated #61 accordingly. It might be a bit of a hack however.

AraHaan commented 3 years ago

Alright merged into my fork.