Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

to-directory-tree doesn't create subdirectories #1633

Closed
timbertson opened this issue Jan 17, 2020 · 23 comments · Fixed by #2437
Closed

to-directory-tree doesn't create subdirectories #1633

timbertson opened this issue Jan 17, 2020 · 23 comments · Fixed by #2437

Comments

@timbertson
Copy link
Collaborator

aside: thanks for for-directory-tree, I'm very excited to use it :D

$ echo '{ a = "hello" }' | dhall to-directory-tree --output issue
$ cat issue/a
hello

$ echo '{ `a/b` = "hello" }' | dhall to-directory-tree --output issue
dhall: issue/a/b: openFile: inappropriate type (Not a directory)

If I mkdir -p issue/a first it works, but that's obviously inconvenient.

While I'm at it, my initial assumption was that to-directory-tree would take a map, instead of a record literal. That seems like it could be more flexible (because the keys can be programmatically generated too), and would also remove the need to support Optional Text (you could just filter out the unwanted items from the map). Should I make a separate issue for that, or does it intentionally not support maps?

@sjakobi
Copy link
Collaborator

sjakobi commented Jan 17, 2020

@timbertson Instead of

$ echo '{ `a/b` = "hello" }' | dhall to-directory-tree --output issue

you can use

$ echo '{ a = { b = "hello" } }' | dhall to-directory-tree --output issue

Does that work for you?

@Profpatsch
Copy link
Member

We should disallow / in the keys for that and tell the user that they should use nested records instead.

@Gabriella439
Copy link
Collaborator

Yeah, I would be fine with changing the to-directory-tree command to disallow / in the keys

@timbertson
Copy link
Collaborator Author

timbertson commented Jan 17, 2020 via email

@sjakobi
Copy link
Collaborator

sjakobi commented Jan 17, 2020

Hehe, that reminds me of https://xkcd.com/1172/. (No offense intended)

@timbertson Could you illustrate a bit what you're doing and how you want to manipulate these file sets in Dhall?

AFAIK, the idea of the to-directory-tree command was that you can use Dhall's record operators (/\, //, .{...}, etc.) to manipulate and combine directory trees / file sets.

@timbertson
Copy link
Collaborator Author

timbertson commented Jan 18, 2020

Sure. It may be that I can use /\ to do everything I need, it just may be fragile when multiple things come into play (recursive merges and overrides).

The basic idea is that I want to package up some common boilerplate, and reference it (via remote import) from many git repos. So (greatly) simplified I might have a Travis.dhall module which provides:

{
, `.travis.yaml` = "TODO"
, `script/lint.sh` = "TODO"
}

The script/lint.sh path would be referenced in .travis.yaml, so the user of this module shouldn't be responsible for choosing where that file goes.

Then say I've got a Docker.dhall module which provides:

{
, `Dockerfile` = "TODO"
, `script/compile.sh` = "TODO"
}

Again, the script path is used by the Dockerfile, so it should all be under control of this module.

To bring together the final set, I was doing ./Travis.dhall // ./Docker.dhall // { ... extra files as a record literal }

I believe I can use nested records and /\. But thinking ahead, I expect I might want some default provided files (for convenience) which some users may wish to override or opt out of. I could do that with, say:

{
, `Dockerfile` = "TODO"
, script = {
  , `compile.sh` = "TODO"
  , `optionalStuff.sh` = Some Text "TODO"
  }
}

Which then makes it quite difficult to combine and override:

let dockerStuff =
  let base = ./Docker.dhall
  in
  base // { script = base.script // { `optionalStuff.sh` = None Text } }

in
./Travis.dhall /\ dockerStuff /\ { ... extra files as a record literal }

So it all seems possible, but:

  • Having to manually merge multiple levels with // to override is awkward
  • Knowing when to use // vs /\ is not obvious (using /\ when you want to override will at least give you an error, but doing the opposite will result in silently omitting files in any overridden subdirectory (script/ in this case))

Flat records with directory-separated paths makes all this much simpler, because the data structure is flat. I can see how it's awkward to have two representations for the same thing though. So perhaps this is just another use case for dhall-lang/dhall-lang#448 or dhall-lang/dhall-lang#340

@sjakobi
Copy link
Collaborator

sjakobi commented Jan 18, 2020

Thanks for the great explanation!

  • Having to manually merge multiple levels with // to override is awkward

Indeed. Lenses should eventually make this easier but they're not here yet.

  • Knowing when to use // vs /\ is not obvious (using /\ when you want to override will at least give you an error, but doing the opposite will result in silently omitting files in any overridden subdirectory (script/ in this case))

Fair point. IMHO it's generally good to start with /\, and to use // only when fields must be overridden. You could also give record completions a try.

I can see how it's awkward to have two representations for the same thing though.

Yes, I think that would make things more complicate down the road.

I think you should give nested records a try for now. If that turns out to be too inergonomic or cause other problems, please report back, and we'll try to find a solution.

@timbertson
Copy link
Collaborator Author

timbertson commented Jan 18, 2020 via email

@Gabriella439
Copy link
Collaborator

@timbertson: For what it's worth, I think we might be able to support deep overrides after all and I have a comment related to that here: dhall-lang/dhall-lang#754 (comment)

@bfrk
Copy link

bfrk commented Mar 12, 2020

@timbertson wrote:

While I'm at it, my initial assumption was that to-directory-tree would take a map, instead of a record literal. That seems like it could be more flexible (because the keys can be programmatically generated too), and would also remove the need to support Optional Text (you could just filter out the unwanted items from the map). Should I make a separate issue for that, or does it intentionally not support maps?

+1 from me. I was just going to open an issue to request this feature.

@sjakobi
Copy link
Collaborator

sjakobi commented Mar 12, 2020

@timbertson, @bfrk By "map" you mean this type, right?

List { mapKey : Text, mapValue : Text }

@bfrk
Copy link

bfrk commented Mar 12, 2020

Yup. Also known as https://prelude.dhall-lang.org/Map/Type

@harris-chris
Copy link

This is a very useful feature, but I seem to be hitting a bit of a limitation:

  • if you're passing in a record, the keys are set at the type level, so you can't have an arbitrary or programmatically defined number of files/directories at any given level of your structure
  • but if you're passing in a fileMap, then every item in the list of the fileMap has to have the same type, therefore every branch of the directory structure has to have the same depth
    I have a function which would ideally generate a config folder which looks a bit like this:
config
|
| -> sub-config-1
|         |
|         | -> _x many files_
|
| -> sub-config-2
          | 
          | -> sub-config-2-1
                    |
                    | -> _y many files_

where x and y vary across different calls of the function. I don't think this is possible from a single dhall to-directory-tree call because of the limitation above. Up until now I've been calling the function multiple times but this is quite slow (it takes a number of minutes to generate all the config).

@Gabriella439
Copy link
Collaborator

@harris-chris: Do you have a specific Dhall type in mind that would represent your desired directory tree? The reason I ask is that I can extend dhall to-directory-tree to support additional Dhall types for this purpose

@harris-chris
Copy link

@harris-chris: Do you have a specific Dhall type in mind that would represent your desired directory tree? The reason I ask is that I can extend dhall to-directory-tree to support additional Dhall types for this purpose

Thanks for the response. I'm sure you'd have a better idea than I, but I can't really see a particularly good solution there, at least not without a heterogenous list type. If dhall to-directory-tree accepted a fileMap in the following format then that would also work:

[ 
    { mapKey = "config/sub-config-1/file1", mapValue = "file 1 contents" }
    , { mapKey = "config/sub-config-2/sub-config-2-1/file2", mapValue = "file 2 contents" }
]

However I'd agree with the comments above, that that approach is a bit clunky compared to having nested records.

For the real-life situation I have here, I've just contrived the structure of the config files to have the same depth across the board, which is a perfectly fine work-around.

@Gabriella439
Copy link
Collaborator

What if dhall to-directory-tree were to support Prelude.JSON.Type? Would that work for you?

@harris-chris
Copy link

That sounds like it would be a convenient solution, yes. So am I right in thinking that from bash I'd do something like, say:

$ dhall to-directory-tree --output my-directory-tree | dhall-to-json "(./file-containing-a-normal-record-expression.dhall)"

@Gabriella439
Copy link
Collaborator

What I mean is that the input would be a Dhall expression of this type:

https://store.dhall-lang.org/Prelude-v21.1.0/JSON/Type.dhall.html

… but you could create a value of that type using json-to-dhall if you wanted to

@harris-chris
Copy link

harris-chris commented Jul 3, 2022 via email

@mmhat
Copy link
Collaborator

mmhat commented Jul 5, 2022

I'd vote for something like:

forall (a : Type) ->
  let EntryType = < Directory : List a | File : Text >

  let Entry = { name : Text, type : EntryType }

  in  forall (make : Entry -> a) -> List a

I'd also like to extend the Entry with fields like user : < Id : Natural | Name : Text > and permissions : Text - But that's a story for another issue.

@harris-chris
Copy link

Hello - another observation about this feature - is there a particular reason that it can't be used to create empy folders? For example,

dhall to-directory-tree <<< "[ { mapKey = \"empty_folder\", mapValue = [] : List { mapKey : Text, mapValue : Text } } ]" --output testing

is an invalid expression for to-directory-tree. Setting mapValue to None will create an empty testing folder, but with no empty_folder sub-folder.

@mmhat
Copy link
Collaborator

mmhat commented Aug 12, 2022

@harris-chris A workaround for that looks like:

let empty_directory = { empty = None Text }
in
{ this_is_an_empty_directory = empty_directory }

Admitted, this is not a nice solution but at least it works. I proposed a more sophisticated option for to-directory-tree in #2436. An implementation draft of that idea is here; Feel free to give that a try.

@harris-chris
Copy link

harris-chris commented Aug 13, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants