Open haileys opened 12 months ago
Oh! I see where those git repo methods are being called after all - in esp-idf-sys
!
I prepared some changes for esp-idf-sys to update according to this change: https://github.com/esp-rs/esp-idf-sys/compare/master...haileys:esp-idf-sys:esp-idf-no-git-repo
It's unfortunate that we'll need to make both of these changes in lockstep, but I can't think of a better way. esp-idf-sys is unfortunately currently coupled to Git repo support - but only really when using a managed repo, otherwise it only calls worktree()
.
If this change looks good, let me know and I'll open up a PR to esp-idf-sys with my linked changes.
@N3xed would you have time to review and provide guidance?
Thank you for the review @N3xed! I have made all the requested changes!
@haileys Changes look good! Just cargo fmt
and maybe cargo clippy
left. If you want you can already make a PR to esp-idf-sys with embuild patched to this branch.
cargo fmt applied!
cargo fmt applied!
There's one clippy error that needs fixing too.
@haileys I'm preparing a new release of embuild
and esp-idf-sys
. Can you address the remaining clippy error so I can merge this for the release? Thanks!
Whoops sorry for the delay, fixed that up now.
@haileys Are you looking at the CI pipeline when you are pushing (not sure - I'm genuinely asking, as I don't remember if the CI pipeline is visible for first-time committers to the project)?
Anyway - you need to run cargo fmt
again.
@haileys Are you still interested in pushing this to completion? We are missing a cargo fmt
in your PR.
embuild currently expects the esp-idf source tree (set by
EMBUILD_ESP_IDF_PATH
) to be a Git repository. It errors out if this path is not a Git repository.This has the unfortunate side effect of preventing esp-idf from being a Git submodule of another repository - this is how I'd like to configure my project, so I can manage esp-idf versions directly and have an easily accessible checkout.
This pull request does two things:
Add a new
SourceTree
enum with a single variant,Repository
. There is aSourceTree::path
method that delegates togit::Repository::worktree
. It appears that this is the only method embuild currently calls ongit::Repository
Adds a second
SourceTree::Plain
variant which holds aPathBuf
directly. This variant is constructed if we fail to open a git repository at the esp-idf path.I wonder if we could just remove the use of
git::Repository
entirely here, but I figure maybe there might be future uses planned, which is why I've opted for the enum variant approach in this PR. My hope is that this change makes embuild more flexible for different project configurations.