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

Makefile: touch lib directory after installing dependencies #380

Merged
merged 1 commit into from
May 12, 2020

Conversation

waj
Copy link
Member

@waj waj commented May 4, 2020

This is to avoid the situation where, if the shard.lock has a mtime more recent than lib directory, the dependencies are installed every time. Is there a better solution for this?

@j8r
Copy link
Contributor

j8r commented May 4, 2020

Why the lib directory mtime matters? It shouldn't, shards should check the version of each library inside this directory each time shards install is run.

@waj
Copy link
Member Author

waj commented May 4, 2020

Of course it does. But I don’t want to see the action being executed every time.

@straight-shoota
Copy link
Member

@j8r This is not about shards' behaviour when installing dependencie. It just fixes the Makefile recipe for lib in this repo.

@waj
Copy link
Member Author

waj commented May 4, 2020

Oh... sorry, the title was misleading

@waj waj changed the title Touch lib directory after installing dependencies Makefile: touch lib directory after installing dependencies May 4, 2020
@j8r
Copy link
Contributor

j8r commented May 4, 2020

@straight-shoota yeah, I don't understand why shards take into account the mtime of lib and shard.lock?

@jhass
Copy link
Member

jhass commented May 4, 2020

@straight-shoota yeah, I don't understand why shards take into account the mtime of lib and shard.lock?

It does not, make does.

Copy link
Member

@jhass jhass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't be surprised if this is the initial reason for touch to exist :D

@straight-shoota
Copy link
Member

straight-shoota commented May 4, 2020

@j8r When you run a make tasks like bin/shards that depends on lib, make checks the mtime of the target (i.e. the lib folder). If that is older than its dependencies (shard.lock), it executes the recipe for lib. That can be avoided by updating mtime every time the recipe runs. Running touch on the target is a typical solution in Makefiles when the recipe itself doesn't update the mtime.

@repomaa
Copy link
Contributor

repomaa commented May 5, 2020

maybe related to #381 ?

@straight-shoota
Copy link
Member

Yeah #381 seems like a good alternative. However, it doesn't fix the problem: If shard.yml was updated with no changes relevant to dependency resolution, then shard.lock will retain an older mtime and the recipe for shard.lock will run shards update every time.

But this could maybe be solved if #381 touches shard.lock even when it didn't change but is older than shard.yml.

@straight-shoota
Copy link
Member

Even shards update doesn't touch lib. Feels like it clearly should.
I'm not entirely sure about shards install with no changes. But update should definitely.

@waj waj merged commit 21c2da8 into crystal-lang:master May 12, 2020
@waj waj deleted the fix/makefile-touch-lib branch May 12, 2020 20:59
@bcardiff bcardiff added this to the v0.11.0 milestone May 27, 2020
taylor pushed a commit to vulk/shards that referenced this pull request Aug 11, 2020
f-fr pushed a commit to f-fr/shards that referenced this pull request Jan 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants