fix: file labels in GitLab releases, to ensure they're unique. by EvanCarroll · Pull Request #267 · semantic-release/gitlab

EvanCarroll · 2021-07-29T18:26:13Z

Previously GitLab lables were just the basename for files uploaded as
part of the release. This is problematic because GitLab doesn't allow
conflicting labels -- a condition that could be caused by uploading a
release with two files by the same name in different directories. This
would generate a 409 Conflict error.

This changes the labels for files uploaded as part of a release to the
name relative to pkgRoot, or the package.

A project may look like this

pkg
pkg \ foo \ baz
pkg \ bar \ baz

This would previously result in two conflicting labels of 'baz'. Now you
would have {"foo/baz", "bar/baz"} with no conflict.

GitHub issues: #265, #158

EvanCarroll · 2021-07-29T19:08:59Z

The force pushes are all commit message. I linked it to the issues and fixed some typos.

EvanCarroll · 2021-07-29T20:40:09Z

It may be worth documenting too, but you would use this feature like this

    ["@semantic-release/gitlab", {
      "gitlabUrl": "https://gitlab.awe.eco.cpanel.net/",
      "pkgRoot": "dist/cpanel/angular-validators",
      "assets": [
        {"path": "dist/cpanel/angular-validators/**", "label": "Dist output"}
      ]   
    }]

Now all things found in dist/cpanel/angular-validators/ have their label in the release relative to the pkgRoot

Previously GitLab lables were just the basename for files uploaded as part of the release. This is problematic because GitLab doesn't allow conflicting labels -- a condition that could be caused by uploading a release with two files by the same name in different directories. This would generate a 409 Conflict error. This changes the labels for files uploaded as part of a release to the name relative to pkgRoot, or the package. A project may look like this pkg pkg \ foo \ baz pkg \ bar \ baz This would previously result in two conflicting labels of 'baz'. Now you would have {"foo/baz", "bar/baz"} with no conflict. GitHub issues: semantic-release#265, semantic-release#158

EvanCarroll · 2021-09-24T18:56:21Z

I rebased this can I get some input on it?

fgreinacher · 2023-10-05T15:19:36Z

This PR has been stale for nearly two years now, so I'm closing it.

Feel free to send a new PR if you want this 🙇

EvanCarroll · 2023-10-05T15:33:34Z

It was stale because you never gave any input on it lol. Despite being asked by others in the same issues #265 (comment)

fgreinacher · 2023-10-05T16:06:08Z

Ha wow, too ambitious in cleaning up. Sorry!

@EvanCarroll I'll review tomorrow or early next week. Can you rebase once more? 🙇

fgreinacher · 2023-10-10T09:31:15Z

lib/glob-assets.js

                // - other properties of the original asset definition
                const {filepath, ...others} = asset;
-                return globbed.map(file => ({...others, path: file, label: basename(file)}));
+                return globbed.map(file => ({...others, path: file, label: path.relative(pkgRoot || '.', file)}));


Should we consider this a breaking change?

@fgreinacher I wouldn't unless generating duplicate labels was support somewhere and things depended o nit. Honestly don't know. Seems weird. Imagine creating a listing of files in multiple directories and losing the directory they were in, that was the old behavior. So the labels would collide and the SCM would reject the operation.

Repo maintainers can decide if the change is breaking instead of hanging that on the MR author. It probably is breaking because previously failing pipelines will begin passing.

fgreinacher · 2023-10-10T11:21:43Z

lib/resolve-config.js

      : 'https://gitlab.com');

  return {
+    pkgRoot,


I understand you are mirroring the NPM plugin here, but the concept of a package does not really exist for the GitLab plugin.

What about assetBasePath or assetRootPath?

Also please add some documentation 🙇

There's no documentation anywhere else in this module. That's a separate issue.

fgreinacher · 2023-10-10T11:23:43Z

test/resolve-config.test.js

+      pkgRoot: undefined,
    }
  );
 });


Can you add some test cases for the new option? Especially weird cases like when the asset is not within the base/root path or when the base/root path is somehow malformed/does not exist.

fgreinacher · 2023-10-10T11:25:21Z

lib/glob-assets.js

+module.exports = async ({pkgRoot, cwd}, assets) => {
+  return uniqWith(


This can stay an arrow function, no?

@fgreinacher I'm confused. It's still an arrow function

I think the nit is because this introduces a new block with {}

fgreinacher

Thanks @EvanCarroll and sorry that I somewhat missed that this is ready for review.

I left some comments :)

fgreinacher · 2023-12-04T13:08:04Z

@EvanCarroll Do you still want to tackle this? If not someone else could take over.

JonasSchubert · 2024-03-03T11:44:20Z

@EvanCarroll shall somebody else take over for you or do you want to continue here?

fgreinacher · 2024-05-24T06:32:22Z

I think somebody else needs to take over here.

EvanCarroll · 2024-05-24T12:47:49Z

Please, take it over. I'm not adding tests for this. If you want to add tests to a project that didn't previously have them, that's another issue imho.

Besides I have to maintain a forks of all of the bugs I've fixed in the semantic-release projects anyway.

EvanCarroll force-pushed the pkgroot branch 2 times, most recently from b33ed93 to 9be4ada Compare July 29, 2021 19:08

EvanCarroll force-pushed the pkgroot branch from bc284ca to 04f20f2 Compare September 24, 2021 18:55

dimooze mentioned this pull request Jan 11, 2022

Can not create release if patterns matches multiple files with the same name. #265

Open

fgreinacher closed this Oct 5, 2023

fgreinacher reopened this Oct 5, 2023

fgreinacher self-requested a review October 5, 2023 16:06

fgreinacher reviewed Oct 10, 2023

View reviewed changes

fgreinacher requested changes Oct 10, 2023

View reviewed changes

		module.exports = async ({pkgRoot, cwd}, assets) => {
		return uniqWith(

Conversation

EvanCarroll commented Jul 29, 2021 • edited by hearts bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EvanCarroll commented Jul 29, 2021

Uh oh!

EvanCarroll commented Jul 29, 2021

Uh oh!

EvanCarroll commented Sep 24, 2021

Uh oh!

fgreinacher commented Oct 5, 2023

Uh oh!

EvanCarroll commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fgreinacher commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgreinacher Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgreinacher Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fgreinacher left a comment

Choose a reason for hiding this comment

Uh oh!

fgreinacher commented Dec 4, 2023

Uh oh!

JonasSchubert commented Mar 3, 2024

Uh oh!

fgreinacher commented May 24, 2024

Uh oh!

EvanCarroll commented May 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

EvanCarroll commented Jul 29, 2021 •

edited by hearts bot

Loading

EvanCarroll commented Oct 5, 2023 •

edited

Loading

fgreinacher commented Oct 5, 2023 •

edited

Loading

fgreinacher Oct 10, 2023 •

edited

Loading

fgreinacher Oct 10, 2023 •

edited

Loading