aspnet / LibraryManager

MIT License
458 stars 83 forks source link

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

Open Valks opened 5 years ago

Valks commented 5 years ago

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": "bootstrap@4.3.1",
      "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

NickDarvey commented 4 years ago

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 commented 4 years ago

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));
Valks commented 4 years ago

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.

lonix1 commented 3 years ago

@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?