Skip to content

Dependency management #251

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

Closed
wants to merge 4 commits into from
Closed

Dependency management #251

wants to merge 4 commits into from

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Nov 24, 2020

Opening this for discussion and feedback.

For a detailed documentation have a look at src/fpm/dependency.f90

  • implement fpm-update command, will fetch all dependencies when called
  • to force an update the --all flag can be used
  • fine grain control over which dependencies are fetched is available by invoking with the dependency names
  • use TOML data structure as “hash table” for dependency management
  • dependencies are stored in build/cache.toml, internal file for now, not part of the user API
  • retrieve and store checked out revision in cache file
  • report unused or orphaned dependencies (don't delete them?)
  • find a way to actually test all this without requiring to actually fetch something
  • more documentation

Requires #247
Related #121

@LKedward
Copy link
Member

Thanks @awvwgk, implementation looks nicely done. Apologies I'm not very familiar with cargo and similar programs so I have a few basic questions:

  • does fpm update only update a dependency if the revision hasn't been specified with a commit or a tag in the manifest?
    • when I try fpm update with a dependency with no version information in the manifest, I notice that no git-obj is produced in the dep-lock.toml - shouldn't the latest commit be stored here?
  • can I use fpm update to update dependencies after I've change the version information in the manifest?
  • does the existing checkout procedure for dependencies need updating to use information from the lock file if it exists?

@awvwgk
Copy link
Member Author

awvwgk commented Nov 26, 2020

Thanks for testing, I'm still experimenting with a useful lock file format and possible update procedure. Right now the lock file is only descriptive to see how reusing information between two separate runs can work and acts more like a hash table to avoid having several versions of the same dependency around.

Getting the actual commit after fetching a git dependency is still something to do.

Finding a good fpm package to test is also quite a challenge, I use jsonff from @everythingfunctional, which has a moderately extensive dependency tree (there the current --all option fails, so I have to redo this part).

- fpm-update will fetch all dependencies when called
- to force an update the `--all` flag can be used
- dependencies are stored in build/cache.toml
@awvwgk
Copy link
Member Author

awvwgk commented Nov 28, 2020

I think I found a stable way to actually handle the dependency update for now. I renamed the dep-lock.toml to cache.toml, since it is currently used as cache rather than an actual lock file.

The update command supports three modes now:

  1. just a plain fpm update will update all dependencies of the project
  2. with fpm update <name>... all the provided dependency names are updated based on the information in the cache file
  3. fpm update --fetch-only will only fetch not already present dependencies, this mode will be used by all other commands interacting with dependencies

The more difficult part will be the creation of unit tests for this, without actually requiring to fetch some git repositories.

@awvwgk awvwgk marked this pull request as ready for review November 28, 2020 15:17
@awvwgk
Copy link
Member Author

awvwgk commented Nov 28, 2020

@LKedward I'm not sure if I want to apply the fetch-only mode already for fpm-build since this might collide with #248, but I would definitely implement the same backend in a later PR or after #248 is merged.

@everythingfunctional
Copy link
Member

There are two ways to design such code to make it unit testable. In a functional style, the code returns the commands/actions it would like to be executed, and some other code further up the call chain is responsible for actually executing them. In this way the unit tests can check that the code is returning the correct actions.

In an object oriented style, the code makes use of an abstract derived type to execute the necessary commands/actions. In the "production" code the caller supplies a version which just executes the actions, but in the test code a version is provided which only records the actions without executing them so that the test can check them.

@LKedward LKedward self-requested a review November 29, 2020 18:40
@awvwgk
Copy link
Member Author

awvwgk commented Dec 2, 2020

I'll give this another try. I'll keep the command-line interface but use an iterative implementation rather than a recursive one.

@awvwgk awvwgk closed this Dec 2, 2020
@awvwgk awvwgk mentioned this pull request Dec 2, 2020
6 tasks
@awvwgk awvwgk deleted the update branch December 10, 2020 18:50
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 this pull request may close these issues.

3 participants