LibraryManager icon indicating copy to clipboard operation
LibraryManager copied to clipboard

Is there a way to set dependencies or order of execution?

Open Valks opened this issue 6 years ago • 4 comments

Functional impact

I have a project where I download libraries i.e. bootstrap and then want to use the filesystem provider to move files around. Problem is because the files haven't downloaded before the filesystem provider triggers I get an error

Minimal repro steps

Create a new project and use the following configuration for libman.json

{
  "version": "1.0",
  "defaultProvider": "cdnjs",
  "libraries": [
    {
      "provider": "unpkg",
      "library": "[email protected]",
      "destination": "Resources/Libraries/bootstrap"
    },
    {
      "provider": "filesystem",
      "library": "Resources/Libraries/bootstrap/dist/css",
      "destination": "content/lib/bootstrap/css",
      "files": [
        "bootstrap.min.css",
        "bootstrap.min.css.map"
      ]
    },
    {
      "provider": "filesystem",
      "library": "Resources/Libraries/bootstrap/dist/js",
      "destination": "content/lib/bootstrap/js",
      "files": [
        "bootstrap.min.js",
        "bootstrap.min.js.map"
      ]
    }
  ]
}

If the files don't already exist it will fail.

Expected result

Ideally I'd like there to be an order for providers, i.e. External (web) providers trigger first then local filesystem providers.

Or a way to add a dependency based on other libraries before executing i.e. "depends": [ "unpkg:bootstrap" ]

Actual result

Depending on if the package has already restored it will either succeed or fail. Notice this issue because it broke when I compiled on the CI server

Valks avatar Mar 08 '19 21:03 Valks

Yeah, I have the exact same issue. Did you find a workaround? I think I might just fork and fix the order of the provider factories to put the file provider last: https://github.com/aspnet/LibraryManager/blob/6ee2979e1e90b0b5f58fa21e4eb303dbb0c1b987/src/LibraryManager.Build/Contracts/Dependencies.cs#L51-L56


Edit: Ok, that didn't work. Must be something else effecting the order.

NickDarvey avatar May 24 '20 02:05 NickDarvey

It fails because of this block: https://github.com/aspnet/LibraryManager/blob/285984f683c213c7f50dfcdb6542fac761e82cf3/src/LibraryManager/Providers/FileSystem/FileSystemCatalog.cs#L150-L153

Which is invoked during validation in the MSBuild task: https://github.com/aspnet/LibraryManager/blob/6ee2979e1e90b0b5f58fa21e4eb303dbb0c1b987/src/LibraryManager.Build/RestoreTask.cs#L72-L79

A potential workaround is to build your own version of the nupkg with a small edit to pass in true as the 'underTest' parameter during the construction of the FileSystemCatalog here (which will mean the directory isn't tested): https://github.com/aspnet/LibraryManager/blob/9c91cdec243777fa5a3ec7714dd634e08604c5d9/src/LibraryManager/Providers/FileSystem/FileSystemProvider.cs#L48 becomes

        return _catalog ?? (_catalog = new FileSystemCatalog(this, true));

NickDarvey avatar May 24 '20 03:05 NickDarvey

Forgot I even raised this issue. Nice that you went through and figured out the problem.

I've since changed my workflow and this is no longer an issue but that doesn't mean it's not worth fixing.

I just had a quick look out of curiosity and the quick and easy way would be to use ISettings through HostInteractions to check for a property to say bypass file check and add it to the if statement: https://github.com/aspnet/LibraryManager/blob/285984f683c213c7f50dfcdb6542fac761e82cf3/src/LibraryManager/Providers/FileSystem/FileSystemCatalog.cs#L150-L153

That said the only time this should be an issue is when LibraryManger is restoring packages so expanding with a depends array that checks the library is configured would be a nice bit to add.

Might be worth firing up a PR with what you think is best and seeing what feedback you get.

Valks avatar May 24 '20 14:05 Valks

@Valks

I've since changed my workflow and this is no longer an issue

How did you get around this problem, or did you change to a different tool altogether?

lonix1 avatar Feb 03 '21 10:02 lonix1