require bug

[didactic-drunk]

2 of the requires are questionable and one is missing (based on foo.cr may be placed under foo/)

# Directly in the `lib` dir.  Is this correct or documented anywhere? 
249511 access("lib/crypto-secret.cr", F_OK) = -1 ENOENT (No such file or directory)
# Outside of `src`?  
249511 access(".../sodium.cr/lib/crypto-secret/crypto-secret.cr", F_OK) = -1 ENOENT (No such file or directory)
# Ok
249511 access(".../sodium.cr/lib/crypto-secret/src/crypto-secret.cr", F_OK) = -1 ENOENT (No such file or directory)

# Missing the subdir check
".../sodium.cr/lib/crypto-secret/src/crypto-secret/crypto-secret.cr

Crystal 1.0.0 [dd40a2442] (2021-03-22)

LLVM: 10.0.0
Default target: x86_64-pc-linux-gnu

Shards 0.14.1 [778875e] (2021-04-21)

[jhass]

I don't follow, can you explain more? A minimal example repository might help.

[kimburgess]

There's a nice explanation of the require behaviour here.

You can override the base path used, but to view the default run crystal env CRYSTAL_PATH. This likely includes a path to std lib on your system as well as lib. With those rules and this default path, the behaviour you are seeing appears to be correct.

[didactic-drunk]

Missing require

There's a nice explanation of the require behavior here.

From the link provided:

If a directory named "filename" is found and it contains a file named "filename.cr" directly underneath it, it is required.

I have
require "crypto-secret"
Where is the check for "lib/crypto-secret/src/crypto-secret/crypto-secret.cr" as described?

@jhass Project link
The file src/crypto-secret.cr is a workaround I added. Delete that file to see the behavior I describe.

Note: This behavior only happens when requiring "crypto-secret" from a shard. It works correctly as described in the documentation when used within the project itself.

Extra require

I thought lib was managed by shards and contained either a set of directories or links to required shards.
If so is this by design or error?
access("lib/crypto-secret.cr", F_OK) = -1

Perhaps it's harmless checking for one extra file that shouldn't exist.

Another extra require

access(".../sodium.cr/lib/crypto-secret/crypto-secret.cr", F_OK)

Shouldn't it only look for lib/shard-name/SRC/filename? (It does look there next)
But why does it look outside of the src dir? Can you have code there? Should you have code there?
Or is it an artifact of a reused method call that's valid elsewhere but not here?

2 fishy opens that aren't described and one missing open make me think requiring shards is a bit off even if it works most of the time.

[straight-shoota]

@didactic-drunk Can you please provide a clear description of the problem with a step-by-step explanation that allows anyone to recreate the problem you're talking about. I have honestly no clue what is supposedly wrong here.

[docelic]

I believe @didactic-drunk did a require "crypto-secret" and then looked with strace to see all the paths that Crystal checks when searching for files. And from all the paths he saw, he is asking about these two patterns, which seem to be out of place:

249511 access("lib/crypto-secret.cr", F_OK)
249511 access(".../sodium.cr/lib/crypto-secret/crypto-secret.cr", F_OK)

But, @didactic-drunk , @kimburgess is correct in saying that this behavior is documented. From the URL referenced, these parts are relevant:

(1) "By default the require path is the location of the standard library that comes with the compiler, and the "lib" directory relative to the current working directory"

(2.1) "If a file named "filename.cr" is found in the require path, it is required."
(This covers the first lookup directly into lib/)

(2.2) "If a directory named "filename" is found and it contains a file named "filename.cr" directly underneath it, it is required."
(This covers the second lookup, which is not within src/)

It is true that in standard cases these files are not found at these specific locations but somewhere else. But the behavior is documented. I'd think this is not creating real problems because it is happening at compile time rather than runtime.

As for the other part of your question about a "missing subdir check" or "missing open", why would it need to pre-check the existence of dirs (making extra syscalls) when it can just count on access(...path...) returning ENOENT if a file isn't there for any reason?

The lookup in shards may look "a bit off" as you said, especially if you observe the least probable paths like the two you've shown, but in effect it is checking for more paths than necessary (rather than less).

[didactic-drunk]

@doecelic is correct about requireandstrace`.

I think my expectations are different based on reading the require documentation. The "2nd rule example" shows "you can have it like this"

- project
  - src
    - file
      - file.cr (requires "./*")
      - sub1.cr
      - sub2.cr

And you can everywhere except when including a shard.

If the require path was glob("lib/*/src") instead of lib

• I think the example in the documentation would work
• The extra requires would go away
• It would be more ruby like
• I'd be able to tell users to `require "compress/zstd" which seems like a more natural/memorable fit for a compression shard.

If anyone has a workaround for the last point I'll take it.

[kimburgess]

The docs are incredibly good, but I always find checking the implementation to be the most accurate way to grok any behaviour. The nice thing with Crystal is you can do that, with any part of the toolchain. In this case, it's here. For non-relative requires this is called for each with each entry in the crystal path as the relative_to.

The 249511 access(".../sodium.cr/lib/ hit seems interesting though. Without seeing the environment where you're requiring this, I can't see a reason for that.

For setting up a shard that houses a set of libs that you want to require separately, I've found the pattern used here to work well too.

[dyslexic-degenerate]

I'm also experiencing this descrepency. From the docs at https://crystal-lang.org/reference/syntax_and_semantics/requiring_files.html#require-filename:

If a directory named "filename" is found with a directory "src" in it and it contains a directory named "filename" directly underneath it with a "filename.cr" file inside it, it is required.

This doesn't appear to be working for shards installed under lib. For example I'd expect the following structure within installed shards to work with a simplistic require based upon the documentation:

require "shardname"

- project-root
  - lib
    - shardname
      - src
        - shardname
          - shardname.cr    

Results in:

 1 | require "shardname"
     ^
Error: can't find file 'shardname'

Placing the shardname.cr as a sibling as follows works:

- project-root
  - lib
    - shardname
      - src
        - shardname
        - shardname.cr    

Am I misudnerstanding the documentation?

[didactic-drunk]

I'm glad at least one other person understands what I'm talking about.

Fix that makes all these problems go away, opens up more flexible include paths and makes it ruby like:
Use glob("lib/*/src") instead of lib for include paths.

[asterite]

It seems the forth rule is incorrect, the docs should be updated.

That said, you can have it structured like this:

- shard
  - src
    - shard.cr # this provides the entire library
    - sub1.cr # provides a subset of the functionality
    - sub2.cr # provides another subset of the functionality

and in turn sub1 and sub2 can be in directories, as @kimburgess explains.

@didactic-drunk I think the main problem everyone (including me) seems to have had with the bug report is that it wasn't clear. A bug report is best in the form:

1. I did this
2. I was expecting this
3. But instead I got this

Instead, the bug report was "This is questionable. Attached is some cryptic output." I had to parse the output to figure it was fstat or some utility that displays which files need to be checked (to this point I don't know what the correct name of the tool is)

Ruby require rules aren't very good in my opinion. I think rubygems changes the load path at runtime for each gem or something like that, which seems like a total hack. The current lookup rule in Crystal tie together a project with the usual lib/shards structure. But that's just my opinion :-)

[straight-shoota]

I thought the fix was easy, just remove the incorrect lookup rule from documentation. But it might not be that simple. The rule is actually correct for paths with multiple components.

foo/bar for examples matches foo/src/foo/bar.cr. That doesn't seem to be mentioned in the docs, though. It's missing in the list for foo/bar/baz expansions in https://crystal-lang.org/reference/syntax_and_semantics/requiring_files.html#other-forms
I think we should check that entire doc page for correctness and completeness.

Back to the original issue here, I think foo should match for foo/src/foo/foo.cr as a combination of foo/bar matching foo/src/foo/bar.cr and foo matching foo/src/foo.cr and foo/foo.cr.

[straight-shoota]

There is also some weird behaviour. For example, foo/bar/baz also matches foo/bar/baz/bar/baz.cr because the nested shard matches just expand the entire path after the first component.

[pseudonym]

Been 4 months, I guess I'll try this again: why does the compiler know about package layout? Shouldn't shards just be setting the correct include path? I can see a good reason to try bar/foo/foo.cr on require "bar/foo", but when you start having complicated heuristics like this, it usually means the division of responsibility is off.

[straight-shoota]

This works completely independent of shards. The compiler just allows placing source files for libraries in lib/#{library_name} to help with code organization. shards plugs into this mechanism and serves as a tool to automatically pull in such libraries as external dependencies. The whole point of this is that shards does not have to tell the compiler about library paths, it just places them where the compiler already looks for them.

These are not heuristics, but well-defined lookup rules. Okay, some individual ones may not be so much well-defined. Those are not that relevant, though. They just need some refinement, but the overall mechanism is pretty clear. And it's definitely not very complicated. The refactoring in #10876 hopefully makes that more visible.

Not sure what your 4 months reference is supposed to mean?

[pseudonym]

I brought it up 4 months ago on another issue and y'all got mad and closed it.

I disagree completely; it looks like it was decided that shards would be stupid and just download things to a directory, and then the compiler was modified whenever someone started using a different package layout to make it work under this system.

Perl, Python, Ruby, GCC.... none of them have a long list of rules for how import names are mapped to paths (Python will look for dir/__init__.py, which is why I said the foo/foo.cr rule could be useful), and none of them look in a randomly-named subdirectory of the current directory.

Shards has much more information than the compiler does and places more restrictions on file layouts, so why must all of this information be passed implicitly through the filesystem instead of shards build prepending the stuff it's downloaded to CRYSTAL_PATH before it exec's the compiler?

Not surprised you're attached to the current behavior, but it seems more optimized for the type of people that build their own compiler, not users. I mean, there's no "make install" command to put the compiler on one's PATH...

[asterite]

pseudonyn: could you explain what's the issue you are facing? We can try to fix it.

[straight-shoota]

Would you be able to provide a link to the previous discussion you mentioned, please? I'd like to know what you're talking about but could not find it.

[pseudonym]

@asterite Despite @didactic-drunk's poor explanation, I'm clearly not the only one who finds the current behavior surprising. This shouldn't be treated as purely a documentation problem, but an opportunity for the behavior to be simplified so it's easier to document.

@straight-shoota crystal-lang/shards#258

[asterite]

Oh, that discussion... I'm still a firm believer that if we make it configurable then if you download some Crystal code or package you have a chance of not knowing how things are organized. But removing the possibility of configuring things, things become simpler and more intuitive. If you used Ruby on Rails you will understand.

[straight-shoota]

Thanks for the link. I'm sorry if you had the impression everyone just completely dismissed your comments. The reason for closing this discussion was just that the original issue had been considered resolved, and when you revived the discussion, it just drew attention to it which results in closing. I tried to explain that to you and asked to open a new issue about the concerns you have.
It just didn't fit in that issue. It's related, but different. Same here, actually. If you want to discuss the Crystal path configuration in general, please open a new discussion about that. Ideally with concrete examples of practical problems with the current approach.
This issue here is about the discrepancy between the documentation and implementation, which can be fixed by changing either to match the other. In any case, that's a minor change.
But potentially overthrowing the entire Crystal path lookup system is a major breaking change, so it has a very different scope. Thanks.