Implement copyDirs parameter, similar to copyFiles #471

buggins · 2014-12-08T14:01:50Z

Should recursively copy directory content to destination.

http://forum.rejectedsoftware.com/groups/rejectedsoftware.dub/thread/4875/

Working ok on dub build. Not sure about visuald/monod generators.

…ly copy directory content to destination

MartinNowak · 2014-12-08T14:19:07Z

source/dub/generators/generator.d

+			{
+				foreach (f; dirEntries(pack_path.toNativeString(), SpanMode.breadth))
+				{
+					foreach (glob; globs)


Well you should definitely add a if (!f.isDir) continue here.

Thank you! Fixed.

MartinNowak · 2014-12-08T14:21:21Z

If you can already use copyFiles with globs, wouldn't that also suffice for dirs.
copyFiles = foo/*
If this is also about subdirs, we should use an extended glob format where ** in foo/**/bar matches an arbitrary number of subdirs.

MartinNowak · 2014-12-08T14:30:05Z

I also wrote a package manager a few years ago, it was called dpk and supported this kind of ** matching.
We could probably reuse globsToRe to extend copyDirs.

buggins · 2014-12-08T15:35:54Z

copyFiles with wildcard is not enough when

need to copy directory content with subdirectories
content should be placed into subdirectory (copyFiles will place all files at the same dir as executable)

…rectories only

MartinNowak · 2014-12-08T17:01:04Z

copyFiles with wildcard is not enough when

need to copy directory content with subdirectories

That's why I suggested wildcard wildcard, which matches across path separators.

content should be placed into subdirectory (copyFiles will place all files at the same dir as executable)

So it's about preserving the output paths. That actually seems like a sane default when using **, that would mean foo/bar/** copies any file under foo into the output dir, preserving it's relative path to foo/bar. Would that be good enough or am I missing something?

buggins · 2014-12-08T17:12:16Z

So it's about preserving the output paths. That actually seems like a sane default when using , that would mean foo/bar/ copies any file under foo into the output dir, preserving it's relative path to foo/bar. Would that be good enough or am I missing something?

Let me try.
If I have

    somePath
        res
            i18n
            mdpi
            hdpi

I need to have res directory located near output file, with all its files and subdirectories like

    executable.exe
    res
         i18n
         mdpi
         hdpi

buggins · 2014-12-08T17:20:08Z

I've tried

    "copyFiles": [
        "res/**"
    ],

It copies all of files from res directory to bin directory (not preserving res dir name). Nested dirs were not copied - errors "cannot copy file" reported for them.

ColdenCullen · 2014-12-08T20:01:09Z

I agree with @buggins. To me, "copyFiles" should be a list of files that get copied, and the globbing simply makes it easier to build that list. The most common use case for "copyFiles" I can think of is this, where you'd want all the files to be along side the executable.

"copyFiles-windows": [
    "bin/*.dll"
]

If you want to preserve folder structure, there really is no good way to configure that. I think "copyDirs" would be a good alternative.

MartinNowak · 2014-12-08T21:44:51Z

It copies all of files from res directory to bin directory (not preserving res dir name). Nested dirs were not copied - errors "cannot copy file" reported for them.

I haven't said that it already works, just suggested to make it work instead of adding an almost identical configuration.

"copyFiles-windows": [
"bin/*.dll"
]

If you want to preserve folder structure, there really is no good way to configure that.

It's really subpathes that we're talking about and currently it's not possible to match them.
Now if we add somethink to match files in subpathes, then we can as well preserve the folder structure.
See http://git-scm.com/docs/gitignore#_pattern_format for example.

So this

"copyFiles-windows": [
    "extra/**.dll"
]

would copy extra/foo.dll and extra/plugins/a.dll to bin/foo.dll and bin/plugins/a.dll.

ColdenCullen · 2014-12-08T22:14:00Z

That certainly seems reasonable, but then how would you specify that you don't want to maintain the subpathes?

Say I have tree:

bin
  + plugins
      - a.dll
  + deps
      - a.dll

a.dll in the deps folder needs to be along side the executable at runtime, but I want to keep the plugins subpath. Unless we implemented a regex-style system? What if the following was valid:

"copyFiles": {
    "plugins/*.dll": "plugins/${0}.dll",
    "deps/*.dll": "${0}.dll"
}

In this example, the left side would be the pattern to match, and the right side would be the destination, substituting any wildcards for ${index} (or anything else, really). This system would not be as friendly to newcomers, but would be much more powerful.

buggins · 2014-12-09T06:00:52Z

"copyFiles": {
    "plugins/*.dll": "plugins/${0}.dll",
    "deps/*.dll": "${0}.dll"
}

In this example, the left side would be the pattern to match, and the right side would be the destination, substituting any wildcards for ${index} (or anything else, really). This system would not be as friendly to newcomers, but would be much more powerful.

This change will break current functionality. In existing package.json files, array is used for this parameter.

buggins · 2014-12-09T06:11:50Z

For me, following fix is enough:

change copyFiles implementation to process directories specified in list in different way than files: copy directory content recursively preserving structure into destination directory (e.g. copyFiles: ["data/res"] when res is a directory will create res directory in output directory.

Currently, copyFiles gives warning "cannot copy file" for directories.

MartinNowak · 2014-12-09T14:28:36Z

For me, following fix is enough:

Thought of that too, and I think it's a good first step.
More complex matchers can always be added later.
When it also copies dirs, we should deprecate copyFiles and come up with a better name.
Any suggestions, just copy isn't bad, but a bit unspecific, copyTree maybe?

buggins · 2014-12-09T14:59:32Z

When it also copies dirs, we should deprecate copyFiles and come up with a better name.
Any suggestions, just copy isn't bad, but a bit unspecific.

I don't see a big problem if we leave it as copyFiles, just need to update documentation.

MartinNowak · 2014-12-10T01:19:49Z

Glad, that we found a better solution :), often worth to be a little patient.
Can you adjust the pull please?

MartinNowak · 2014-12-10T01:31:35Z

That certainly seems reasonable, but then how would you specify that you don't want to maintain the subpathes?

By matching files, plugins/*.dll will copy the DLLs only, plugins would copy the dir.

In this example, the left side would be the pattern to match, and the right side would be the destination, substituting any wildcards for ${index} (or anything else, really).

The double wildcard is already an established solution to solve all of those problems. No need to come up with something new.

This system would not be as friendly to newcomers, but would be much more powerful.

See above, ** is also fairly easy to grasp. It also seems that we don't urgently need something powerful as you could always rearrange your folder structure.

MartinNowak · 2014-12-10T04:57:33Z

That might even be a candidate for std.file.
http://zsh.sourceforge.net/Doc/Release/Expansion.html#Recursive-Globbing

…nts, if directory name is specified in file list (or matched by wildcard)

buggins · 2014-12-10T06:21:26Z

Glad, that we found a better solution :), often worth to be a little patient.
Can you adjust the pull please?

Done. Pull request is updated.
Now copyFiles can accept directory names (including matching by patterns).

Sample source:

+ dir1
    + res
        + mdpi
            - file1.png
            - file2.png
        + hdpi
            - file1.png
            - file2.png
        + i18n
            - resources_en.xml
            - resources_fr.xml

For copyFiles: ["dir1/res"] in output directory following tree will be added:

+ res
    + mdpi
        - file1.png
        - file2.png
    + hdpi
        - file1.png
        - file2.png
    + i18n
        - resources_en.xml
        - resources_fr.xml

For copyFiles: ["dir1/res/*dpi"] in output directory following tree will be added:

+ mdpi
    - file1.png
    - file2.png
+ hdpi
    - file1.png
    - file2.png

MartinNowak · 2014-12-10T07:48:13Z

Wow, that was fast. If you already have a sample project, please turn it into a test case https://github.com/D-Programming-Language/dub/tree/master/test.

MartinNowak · 2014-12-10T07:57:54Z

source/dub/generators/generator.d

+			{
+				mkdirRecurse(dstfolder.toNativeString());
+				foreach (de; iterateDirectory(folder.toNativeString())) {
+					if (de.name.startsWith(".")) continue;


I've never seen dirEntries returning ".", sure it's needs?
http://dpaste.dzfl.pl/fa97e9925d67

buggins · 2014-12-10T08:00:02Z

Wow, that was fast. If you already have a sample project, please turn it into a test case

What should be project directory name under test/ ?
Is it ok to name it 3-copyfiles?

MartinNowak · 2014-12-10T08:02:56Z

Not sure about visuald/monod generators.

Haven't worked with any of the generators yet, but from my understanding all they do is to create a solution file that lists all sources but invokes dub for the actual building. So this should just work.

buggins · 2014-12-10T08:09:26Z

Removed check for directories starting with .
But we need to add this case to tests.

buggins · 2014-12-10T08:41:39Z

Added test for copyFile.

Following cases are being checked:

plain file - exact name
plain files - using wildcards
directory - exact name
directories - using wildcards
directory with name starting with dot - should be skipped

MartinNowak · 2014-12-10T11:50:46Z

~~If I go into that folder and execute dub run twice I get the following error.~~

Sorry for the noise, was using my system-wide dub.

buggins · 2014-12-10T12:14:20Z

If I go into that folder and execute dub run twice I get the following error.

Let me check.

buggins · 2014-12-10T12:22:52Z

Under Windows there is no such issue.

Implement copyDirs parameter, similar to copyFiles

MartinNowak · 2014-12-10T12:23:40Z

Under Windows there is no such issue.

My mistake, see #471 (comment) (maybe hit F5).

buggins · 2014-12-10T12:24:12Z

Thanks!

MartinNowak · 2014-12-10T12:25:00Z

I also compared the actual files with a list of expected files, so that we have a real regression test.
c6a9bbd
Hope that works on Win as well.

MartinNowak · 2014-12-10T12:50:07Z

Could you do me a favor an add a Changelog entry.

buggins · 2014-12-10T14:22:43Z

Could you do me a favor an add a Changelog entry.

Should I do it in separate pull request?

Could not you update it yourself?

MartinNowak · 2014-12-11T00:03:59Z

Could not you update it yourself?

Will do, it also needs to be documented on dub-registry anyhow.

xfront · 2015-03-24T08:55:51Z

"copyFiles-windows": ["tools/win/usb_driver", "tools/win/.dll", "tools/win/.exe"]
will copy usb_driver/*.dll to bin,that is not my expect!

MartinNowak · 2015-03-24T13:26:57Z

It should copy the folder usb_driver to bin, is this a bug?
If so, please report it separately.

s-ludwig · 2015-03-24T13:50:45Z

Hm, "copyDirs": ["tools/win/usb_driver"] should copy the "usb_driver" folder, "copyFiles" just copies individual files. Any more complex operation currently needs to be done with a "preGenerateCommands" entry instead.

We'll have to think about a more sophisticated way to deal with runtime resource files at some point, but that's a relatively complex topic if the goal is to have that work well for all operating systems and applications. A DEP would probably provide the best scope for discussing this.

BTW, I wish I had named "copyFiles" "dllFiles" instead, because it was really never intended to be good enough to handle anything else. Instead of a partial solution that "copyFiles"+"copyDirs" is, I would rather have liked to implement a more sophisticated solution that provides enough information to be useful for helping with automatically generating system packages (DEB etc.), for example.

buggins · 2015-03-24T14:56:31Z

My initial implementation added copyDirs, has been replaced with fix of copyFiles behavior - to copy directory with content, retaining directory name.
To copy w/o directory name, "copyFiles-windows": ["tools/win/usb_driver/*"] may be used

s-ludwig · 2015-03-24T15:00:10Z

I see... I'll have to go and go through the actual code. The new behavior should be documented in https://github.com/D-Programming-Language/dub-registry/blob/master/views/package_format.dt.

MartinNowak · 2015-03-24T21:34:29Z

a more sophisticated solution

The only sophistication still missing is matching across paths using an extended ** glob.

MartinNowak · 2015-03-24T21:43:31Z

"copyFiles-windows": ["tools/win/usb_driver", "tools/win/.dll", "tools/win/.exe"]

You want "copyFiles-windows": ["tools/win/usb_driver/*.dll", "tools/win/*.dll", "tools/win/*.exe"] I think.

MartinNowak · 2015-03-24T22:27:32Z

See dlang/dub-registry#101.

s-ludwig · 2015-03-24T22:35:36Z

a more sophisticated solution

The only sophistication still missing is matching across paths using an extended ** glob.

What I mean are things like copying to a different location than the target directory. The simplest example would be copying to a deeper directory, such as bin/resources/some_folder instead of just bin/some_folder. But it would also be a possible direction to enable copying to system locations, such as /usr/share or C:\ProgramData.

xfront · 2015-03-25T01:22:45Z

the directory structure is:

I want to copy usb_drivers(directory) and adb.exe AdbWinApi.dll to bin.but "copyFiles-windows": ["tools/win/usb_driver", "tools/win/.dll", "tools/win/.exe"] will copy dll in amd64 and i386 too,
that is not my need.

Implement copyDirs parameter, similar to copyFiles - should recursive…

4779426

…ly copy directory content to destination

MartinNowak reviewed Dec 8, 2014
View reviewed changes

fix MartinNowak's finding - wildcard copyDirs should be applied to di…

5023ed0

…rectories only

Modify copyFiles to allow recursive copying of directories with conte…

cf83b22

…nts, if directory name is specified in file list (or matched by wildcard)

MartinNowak reviewed Dec 10, 2014
View reviewed changes

remove unnecessary check for directories starting with dot

bb55812

add test for copyFiles

715d8aa

MartinNowak merged commit 715d8aa into dlang:master Dec 10, 2014

MartinNowak added a commit that referenced this pull request Dec 10, 2014

Merge pull request #471 from buggins/master

a3a6512

Implement copyDirs parameter, similar to copyFiles

MartinNowak mentioned this pull request Mar 23, 2015

dub copyFiles does not support directory and wildcards files! #531

Closed

Implement copyDirs parameter, similar to copyFiles #471

Implement copyDirs parameter, similar to copyFiles #471

Conversation

buggins commented Dec 8, 2014

MartinNowak Dec 8, 2014

Choose a reason for hiding this comment

buggins Dec 8, 2014

Choose a reason for hiding this comment

MartinNowak commented Dec 8, 2014

MartinNowak commented Dec 8, 2014

buggins commented Dec 8, 2014

MartinNowak commented Dec 8, 2014

buggins commented Dec 8, 2014

buggins commented Dec 8, 2014

ColdenCullen commented Dec 8, 2014

MartinNowak commented Dec 8, 2014

ColdenCullen commented Dec 8, 2014

buggins commented Dec 9, 2014

buggins commented Dec 9, 2014

MartinNowak commented Dec 9, 2014

buggins commented Dec 9, 2014

MartinNowak commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

buggins commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

MartinNowak Dec 10, 2014

Choose a reason for hiding this comment

buggins Dec 10, 2014

Choose a reason for hiding this comment

buggins commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

buggins commented Dec 10, 2014

buggins commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

buggins commented Dec 10, 2014

buggins commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

buggins commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

MartinNowak commented Dec 10, 2014

buggins commented Dec 10, 2014

MartinNowak commented Dec 11, 2014

xfront commented Mar 24, 2015

MartinNowak commented Mar 24, 2015

s-ludwig commented Mar 24, 2015

buggins commented Mar 24, 2015

s-ludwig commented Mar 24, 2015

MartinNowak commented Mar 24, 2015

MartinNowak commented Mar 24, 2015

MartinNowak commented Mar 24, 2015

s-ludwig commented Mar 24, 2015

xfront commented Mar 25, 2015