Allow specifying destination per file

[Visne]

Allow specifying destination per file

[Visne]

In my opinion this should work similarly to how the mv command works (in a Bash context). That is, allow moving individual files, or use globbing to move multiple files (without preserving directory structure). Many developers are already very familiar with the syntax for globbing, and it is reasonably powerful.

The config file would then look something like this:

{
	"version": "1.0",
	"defaultProvider": "cdnjs",
	"defaultDestination": "js/lib", // The destination would act similar to a "working directory" for mapping
	"libraries": [
		{
			"library": "someLibrary",
			"mapping": {
				// Basic file placement with renaming
				// Would result in `my/preferred/location/custom_filename.js`
				"cdn/directory/structure/file.js": "my/preferred/location/custom_filename.js",
				
				// Placing the file in a directory without changing the filename (trailing slash required)
				// Would result in `my/preferred/location/file.js`
				"cdn/directory/structure/file.js": "my/preferred/location/",

				// Placing a full directory
				// Would result in alll files in some/dir/ to be placed in my/preferred/location/
				"some/dir/": "my/preferred/location/",
				
				// Using globbing
				// Would result in all .js files in some/dir/ ending up in somewhere/
				// Note that while the trailing slash on somewhere/ would not be required, when globbing is
				// used it is always interpreted as a directory, as globbing can match 0 to infinite files
				"some/dir/*.js": "somewhere/",
				
				// Using directory globbing
				// Would result in all .js files in the some/dir/ and all its subdirectories being moved to somewhere/
				// Note that this could lead to filename collisions, an error should be shown to the user when this 
				// happens telling them what name collision happened and what mapping rule caused it
				"some/dir/**/*.js": "somewhere/"
			}
		}
	]
}

Some special notes I can think of:

• Since backwards compatibilty would be very important for this (otherwise projects could suddenly break after an update) it is probably a good idea to introduce this as an experimental feature first, in case anything turns out to need drastic changes.
• Microsoft already has a globbing library which could probably be used to do a ton of the work.
• One thing that is hard to do with this globbing style is taking all files of a specific type (*.js for example) from a specific directory while keeping the directory structure. Since some/dir/**/*.js would move all .js files to one directory, you would have to use some/dir/*.js and some/other/dir/*.js etc to map all directories individually. I would argue that if you want this functionality but there are too many subdirectories to do this for each individually, then LibraryManager is probably not the tool for you anyways.
• It is somewhat ambiguous what would happen if "mapping" and packages.files are both specified. Perhaps they should just be mutually exclusive, with the user getting an error when both are specified.

[ruslan-mogilevskiy]

@jimmylewis , are there plans to add this feature?

[jimmylewis]

@ruslan-mogilevskiy yes, I'd like to, but I haven't had time. This is a big feature to work out all the details: it has fairly broad impact from changes to the JSON schema, changes to how we pre-validate the manifest (including support for file glob patterns and duplications), and the actual implementation of file placement, so it needs to be clearly spec'ed out. I'd welcome contributions for this, with the caveat of it will likely be fairly involved and my time right now is limited.

Design suggestions would be great too, if there's a way you'd like to see this look in the libman.json file. Having a few ideas or a consensus of what folks would find intuitive and easy to use might help with guiding the code changes.

[jimmylewis]

We already use the minimatch library in libman, glob support was added a while back for the existing scenario. The Microsoft.Extensions.FileSystemGlobbing library has some behavioral differences, but mostly is harder to use because it works directly against the filesystem, which we don't have when computing these matches during pre-validation (we compute the globs against the file metadata, not files on disk).

Here's some edge cases building on your example:

{
  "version": "1.0",
  "defaultProvider": "cdnjs",
  "defaultDestination": "js/lib"m
  "libraries": [
    {
      "library": "fileMappedTwiceExample",
      "mapping": {
        // is it valid to install the same file to two separate locations via separate patterns? (seems like it would be allowed, as long as it doesn't create conflicts)
        "files/foo.js": "somewhere/"
        "**/foo.js": "somewhereElse/"
    },
    {
      "library": "fileMappedTwiceExample2",
      "mapping": {
        // What if I want to duplicate a file?  This isn't valid JSON because the property name is reused.
        "files/foo.js": "somewhere/"
        "files/foo.js": "somewhereElse/"
      }
    },
    {
      "library": "fileConflictsExample",
      "mapping": {
        // We need to ensure no collisions across all files + glob expansions
        "a/*.js": "somewhere/"
        // b/ should not contain any files with the same name as a/
        "b/*.js": "somewhere/"
        // there should not be another bar.js in a/ or b/
        "c/foo.js", "somewhere/bar.js" 
      }
    }
  ]
}

Also, preserving directory structure I think is an important, or at least reasonably common, scenario to try to address. Some packages like bootstrap (when fetched via jsdelivr or unpkg) have a top level "dist" folder, and we've seen this feature requested a number of times to try to collapse that. Something maybe like this?

    {
      "library": "bootstrap@latest",
      "mapping": {
        // trying to remove the "dist" folder
        "dist/**/*": "**/*" // special case - if ** exists on both sides, make the result match the source expansion?
      }
    }

If we do support this, do we try to support complex glob patterns with multiple ** matches (e.g. **/foo/**/*.js)?

Another slightly different approach I was thinking of was to make the mappings be an array of objects. This allows the duplicates in my second example, because they could just be in separate objects, e.g.

{
  "version": "1.0",
  "defaultProvider": "cdnjs",
  "defaultDestination": "js/lib"
  "libraries": [
    {
      "library": "fileMappedTwiceExample2",
      "mappings": [
        {
          "files/foo.js": "somewhere/",
          "styles/*.css": "anotherFolder/"
        },
        {
          "files/foo.js": "somewhereElse/"
          // don't need the same mappings, but this allows duplicating the property name
        }
      ]
    }
  ]
}

Maybe it could be defined as either an array or an object, to enable this special case while keeping it as simple as possible for the majority scenario? Or is that too much of an edge case to worry about supporting?

[Visne]

but mostly is harder to use because it works directly against the filesystem

Yeah, that was a problem I was thinking about but I forgot LibraryManager already uses Minimatch.

is it valid to install the same file to two separate locations via separate patterns? (seems like it would be allowed, as long as it doesn't create conflicts)

Yes, I think all moves should be validated beforehand and if there is a conflict the user should get an error.

What if I want to duplicate a file? This isn't valid JSON because the property name is reused.

That's a good point, I hadn't considered that yet. I think the only good solution to that is to map to an array like so:

"mapping": {
	"files/foo.js": [
		"somewhere/",
		"somewhereElse/"
	]
}

It will probably be slightly annoying to do in C# though because of static typing (assuming the JSON is deserialized to an object).

trying to remove the "dist" folder

Well, don't forget that the example you gave is already trivial to do by just moving the directory, instead of using glob patterns, like so:

"dist/": "."

Still, this does indeed not allow for much freedom and when multiple ** patterns are used this creates problems. So to anwer your follow-up question:

do we try to support complex glob patterns with multiple ** matches

I think that if we really do want this, we should handle it similarly to RegEx, in that you essentially have capture groups that you can refer to later (although globbing is simple enough that capture groups don't have to be explicitly stated, instead all ** and * can become capture groups automatically). Apparently Apache Struts already does this. Using their syntax, the mapping would look like this:

"**/foo/**/*.js": "{2}/"

This would put all .js files that are in a subdirectory of any foo/ directory, into directories matched by the second ** (while maintaining the structure). I think the main argument against this would be that it adds a ton of complexity, while most projects should be small enough that even mapping all files/directories individually would already be easy enough.

make the mappings be an array of objects

I don't really like this, because it keeps the targets of the file you want to duplicate far away from each other, and it will be confusing to developers that don't need to duplicate files. I think the idea I described before would be better, where you optionally give an array of target locations instead of a single target location.

[AraHaan]

Another pain I face:

{
  "version": "1.0",
  "defaultProvider": "jsdelivr",
  "libraries": [
    {
      "provider": "cdnjs",
      "library": "[email protected]",
      "destination": "Scripts/jquery-validation-unobtrusive/"
    },
    {
      "library": "[email protected]",
      "destination": "Scripts/jquery-validation/",
      "files": [
        "dist/additional-methods.js",
        "dist/additional-methods.min.js",
        "dist/jquery.validate.js",
        "dist/jquery.validate.min.js",
        "dist/localization/messages_ar.js",
        "dist/localization/messages_ar.min.js",
        "dist/localization/messages_az.js",
        "dist/localization/messages_az.min.js",
        "dist/localization/messages_bg.js",
        "dist/localization/messages_bg.min.js",
        "dist/localization/messages_bn_BD.js",
        "dist/localization/messages_bn_BD.min.js",
        "dist/localization/messages_ca.js",
        "dist/localization/messages_ca.min.js",
        "dist/localization/messages_cs.js",
        "dist/localization/messages_cs.min.js",
        "dist/localization/messages_da.js",
        "dist/localization/messages_da.min.js",
        "dist/localization/messages_de.js",
        "dist/localization/messages_de.min.js",
        "dist/localization/messages_el.js",
        "dist/localization/messages_el.min.js",
        "dist/localization/messages_es.js",
        "dist/localization/messages_es.min.js",
        "dist/localization/messages_es_AR.js",
        "dist/localization/messages_es_AR.min.js",
        "dist/localization/messages_es_PE.js",
        "dist/localization/messages_es_PE.min.js",
        "dist/localization/messages_et.js",
        "dist/localization/messages_et.min.js",
        "dist/localization/messages_eu.js",
        "dist/localization/messages_eu.min.js",
        "dist/localization/messages_fa.js",
        "dist/localization/messages_fa.min.js",
        "dist/localization/messages_fi.js",
        "dist/localization/messages_fi.min.js",
        "dist/localization/messages_fr.js",
        "dist/localization/messages_fr.min.js",
        "dist/localization/messages_ge.js",
        "dist/localization/messages_ge.min.js",
        "dist/localization/messages_gl.js",
        "dist/localization/messages_gl.min.js",
        "dist/localization/messages_he.js",
        "dist/localization/messages_he.min.js",
        "dist/localization/messages_hr.js",
        "dist/localization/messages_hr.min.js",
        "dist/localization/messages_hu.js",
        "dist/localization/messages_hu.min.js",
        "dist/localization/messages_hy_AM.js",
        "dist/localization/messages_hy_AM.min.js",
        "dist/localization/messages_id.js",
        "dist/localization/messages_id.min.js",
        "dist/localization/messages_is.js",
        "dist/localization/messages_is.min.js",
        "dist/localization/messages_it.js",
        "dist/localization/messages_it.min.js",
        "dist/localization/messages_ja.js",
        "dist/localization/messages_ja.min.js",
        "dist/localization/messages_ka.js",
        "dist/localization/messages_ka.min.js",
        "dist/localization/messages_kk.js",
        "dist/localization/messages_kk.min.js",
        "dist/localization/messages_ko.js",
        "dist/localization/messages_ko.min.js",
        "dist/localization/messages_lt.js",
        "dist/localization/messages_lt.min.js",
        "dist/localization/messages_lv.js",
        "dist/localization/messages_lv.min.js",
        "dist/localization/messages_mk.js",
        "dist/localization/messages_mk.min.js",
        "dist/localization/messages_my.js",
        "dist/localization/messages_my.min.js",
        "dist/localization/messages_nl.js",
        "dist/localization/messages_nl.min.js",
        "dist/localization/messages_no.js",
        "dist/localization/messages_no.min.js",
        "dist/localization/messages_pl.js",
        "dist/localization/messages_pl.min.js",
        "dist/localization/messages_pt_BR.js",
        "dist/localization/messages_pt_BR.min.js",
        "dist/localization/messages_pt_PT.js",
        "dist/localization/messages_pt_PT.min.js",
        "dist/localization/messages_ro.js",
        "dist/localization/messages_ro.min.js",
        "dist/localization/messages_ru.js",
        "dist/localization/messages_ru.min.js",
        "dist/localization/messages_sd.js",
        "dist/localization/messages_sd.min.js",
        "dist/localization/messages_si.js",
        "dist/localization/messages_si.min.js",
        "dist/localization/messages_sk.js",
        "dist/localization/messages_sk.min.js",
        "dist/localization/messages_sl.js",
        "dist/localization/messages_sl.min.js",
        "dist/localization/messages_sr.js",
        "dist/localization/messages_sr.min.js",
        "dist/localization/messages_sr_lat.js",
        "dist/localization/messages_sr_lat.min.js",
        "dist/localization/messages_sv.js",
        "dist/localization/messages_sv.min.js",
        "dist/localization/messages_th.js",
        "dist/localization/messages_th.min.js",
        "dist/localization/messages_tj.js",
        "dist/localization/messages_tj.min.js",
        "dist/localization/messages_tr.js",
        "dist/localization/messages_tr.min.js",
        "dist/localization/messages_uk.js",
        "dist/localization/messages_uk.min.js",
        "dist/localization/messages_ur.js",
        "dist/localization/messages_ur.min.js",
        "dist/localization/messages_vi.js",
        "dist/localization/messages_vi.min.js",
        "dist/localization/messages_zh.js",
        "dist/localization/messages_zh.min.js",
        "dist/localization/messages_zh_TW.js",
        "dist/localization/messages_zh_TW.min.js",
        "dist/localization/methods_de.js",
        "dist/localization/methods_de.min.js",
        "dist/localization/methods_es_CL.js",
        "dist/localization/methods_es_CL.min.js",
        "dist/localization/methods_fi.js",
        "dist/localization/methods_fi.min.js",
        "dist/localization/methods_it.js",
        "dist/localization/methods_it.min.js",
        "dist/localization/methods_nl.js",
        "dist/localization/methods_nl.min.js",
        "dist/localization/methods_pt.js",
        "dist/localization/methods_pt.min.js"
      ]
    },
    {
      "provider": "cdnjs",
      "library": "[email protected]",
      "destination": "Scripts/jquery/"
    },
    {
      "provider": "cdnjs",
      "library": "[email protected]",
      "destination": "Scripts/modernizr/",
      "files": [
        "modernizr.js"
      ]
    },
    {
      "provider": "cdnjs",
      "library": "[email protected]",
      "destination": "Scripts/bootstrap/",
      "files": [
        "js/bootstrap.bundle.js",
        "js/bootstrap.bundle.js.map",
        "js/bootstrap.bundle.min.js",
        "js/bootstrap.bundle.min.js.map",
        "js/bootstrap.esm.js",
        "js/bootstrap.esm.js.map",
        "js/bootstrap.esm.min.js",
        "js/bootstrap.esm.min.js.map",
        "js/bootstrap.js",
        "js/bootstrap.js.map",
        "js/bootstrap.min.js",
        "js/bootstrap.min.js.map"
      ]
    },
    {
      "provider": "cdnjs",
      "library": "[email protected]",
      "destination": "Content/bootstrap/",
      "files": [
        "css/bootstrap-grid.css",
        "css/bootstrap-grid.css.map",
        "css/bootstrap-grid.min.css",
        "css/bootstrap-grid.min.css.map",
        "css/bootstrap-grid.rtl.css",
        "css/bootstrap-grid.rtl.css.map",
        "css/bootstrap-grid.rtl.min.css",
        "css/bootstrap-grid.rtl.min.css.map",
        "css/bootstrap-reboot.css",
        "css/bootstrap-reboot.css.map",
        "css/bootstrap-reboot.min.css",
        "css/bootstrap-reboot.min.css.map",
        "css/bootstrap-reboot.rtl.css",
        "css/bootstrap-reboot.rtl.css.map",
        "css/bootstrap-reboot.rtl.min.css",
        "css/bootstrap-reboot.rtl.min.css.map",
        "css/bootstrap-utilities.css",
        "css/bootstrap-utilities.css.map",
        "css/bootstrap-utilities.min.css",
        "css/bootstrap-utilities.min.css.map",
        "css/bootstrap-utilities.rtl.css",
        "css/bootstrap-utilities.rtl.css.map",
        "css/bootstrap-utilities.rtl.min.css",
        "css/bootstrap-utilities.rtl.min.css.map",
        "css/bootstrap.css",
        "css/bootstrap.css.map",
        "css/bootstrap.min.css",
        "css/bootstrap.min.css.map",
        "css/bootstrap.rtl.css",
        "css/bootstrap.rtl.css.map",
        "css/bootstrap.rtl.min.css",
        "css/bootstrap.rtl.min.css.map"
      ]
    }
  ]
}

This feature could actually help with this where I want all of bootstraps js files under Scripts/bootstrap, while also wanting all of the css files under Content/bootstrap. Currently it does not handle duplicate package definitions well like this.