embuild shouldn't require git or .git to exist in order to build the project
#81
Comments
|
Here's a pull request that was already reviewed and requires just a little love from somebody to get merged. (As in If you would like to own it, we can have this issue fixed. What I would suggest is to clone / fork |
|
And the corresponding PR to |
|
Awesome, I'll give it a shot |
|
@knarkzel Sorry - I've deleted your post. You know it could be anything as to why the previous contributor has not reacted. Let's concentrate on owning and fixing the PR instead. :) |
|
Ait, so I tried out these patches: Output of Still get the same error, am I missing something obvious? Is it this |
|
Need to run this custom path instead, what's the env variable for that? |
|
|
|
|
It still thinks it's managed despite having this. |
|
Well you might have to dig deeper. As in setting |
|
Hmm lets see... |
|
I must be failing to patch something because the PR looks good |
|
Trying this:
Gives: Adding |
|
|
It seems the pull request fixed one issue, but there's several other places that must be fixed |
|
Maybe the relevant question here is... is |
|
By the way... what is it with *nix that I cannot automatically clone a git repo and then just use it? |
|
It's possible, just not recommended. Everything in |
|
I'll take another look later |
|
Got the same issue (with managed disabled) when using shell that lets |
|
This is the shell: {
inputs = {
nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
esp-dev = {
url = "github:thiskappaisgrey/nixpkgs-esp-dev-rust";
inputs.nixpkgs.follows = "nixpkgs";
};
};
outputs = {
self,
nixpkgs,
esp-dev,
}: let
pkgs = import nixpkgs {
system = "x86_64-linux";
overlays = [ esp-dev.overlay ];
};
in {
devShells.x86_64-linux.default = pkgs.mkShell {
buildInputs = with pkgs; [
gcc-riscv32-esp32c3-elf-bin
openocd-esp32-bin
# Tools required to use ESP-IDF.
git
wget
gnumake
flex
bison
gperf
pkg-config
cmake
ninja
ncurses5
python3
python3Packages.pip
python3Packages.virtualenv
];
shellHook = ''
export ESP_IDF_VERSION="v4.4"
export LD_LIBRARY_PATH="${pkgs.lib.makeLibraryPath [ pkgs.libxml2 pkgs.zlib pkgs.stdenv.cc.cc.lib ]}"
export LIBCLANG_PATH=${pkgs.llvmPackages.libclang.lib}/lib
'';
};
};
} |
|
Might've forgotten to remove the patch, trying again.. 😆 |
I think you should try to confirm/deny this first. |
|
Tried it where I let |
|
You need to git clone esp idf ONLY and then try to build something with it. Like a C program. If this does not work out of the box on *nix, whatever else we try to do in the Rust crates won't work either, as they automate exactly that. |
Okay, I'll attempt a validation build of |
|
@ivmarkov I've confirmed some success here. I have cloned I tried to build an example in that repo: The crate itself compiles and locates the IDF_* paths from my environment - however, cmake appears to perform the same Collapsed cargo outputWhat validation would you like to see? Is there a more isolated test I can do that would validate this work before similar changes are made in cmake tooling? |
Before merging this, I would like to see a successful build, even if that means forking the In fact, having a very clear picture what changes (if any? are we 100% sure we need to make changes to We just waited ~ 1 month for a 3-lines' change to make it in there. |
Okay, seems reasonable - I'll dig in more this week and see what I come up with. Hoping my changes can be kept to |
|
@ivmarkov: Okay! I've confirmed that with these changes, I can successfully build the
I used a fresh Ubuntu container that followed the installation instructions from the Rust on ESP book. I've put my build configuration/output in the collapsed blocks below: `IDF_PATH` -> Invalid git repository`IDF_PATH` -> unsetIf you're happy with this, I'll PR these changes. After that, I can also PR the necessary changes to |
|
@denbeigh2000 OK, good! Next step is to open PRs against the embuild and sys repos that contain your changes - once you feel ready with those. Would be good to also explain shortly how you did workaround the failure in the CMake build. If I'm not mistaken, the original plan was to do changes to |
No problem, I'll get those up today.
Sure - I think the actual problem in my initial validation was user error, I misconfigured my environment variables while trying to package my toolchain. 😅 The idea was never to tackle This is the specific error I was hitting in my output: ( Going in further, I found this should only be evaluated if my env vars are unset. Once I realised that, I decided to just test the problem on its own. Confusingly, IDF still attempts to call Turns out compiler toolchains are hard, and solving just one problem at once made life a lot easier 😄 |
|
After doing some more thorough testing, I'd like one more day to consider these changes. Namely, I want to fix the interaction between these functions. Currently, they both need to be aware of if
My changes to I found that I was not invoking When I fixed this, I found that this check in IMHO the nicest way to fix would be applying this change in diff --git a/components/openthread/CMakeLists.txt b/components/openthread/CMakeLists.txt
index 455b346924..5b538c2d57 100644
--- a/components/openthread/CMakeLists.txt
+++ b/components/openthread/CMakeLists.txt
@@ -168,20 +168,26 @@ execute_process(
COMMAND git rev-parse --short HEAD
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
OUTPUT_VARIABLE IDF_VERSION_FOR_OPENTHREAD_PACKAGE OUTPUT_STRIP_TRAILING_WHITESPACE
)
execute_process(
COMMAND git rev-parse --short HEAD
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/openthread
OUTPUT_VARIABLE OPENTHREAD_VERSION OUTPUT_STRIP_TRAILING_WHITESPACE
)
+if (STATUS AND NOT STATUS EQUAL 0)
+ # If this is not a valid git checkout, eagerly set OT_PACKAGE_VERSION for openthread.
+ # If this is unset, openthread's compilation process will invoke git to set this variable,
+ # causing a build failure.
+ set(OT_PACKAGE_VERSION "development" CACHE STRING "OpenThread Package Version")
+endif()
string(TIMESTAMP OT_BUILD_TIMESTAMP " %Y-%m-%d %H:%M:%S UTC" UTC)
string(CONCAT OT_FULL_VERSION_STRING
"openthread-esp32/"
"${IDF_VERSION_FOR_OPENTHREAD_PACKAGE}-${OPENTHREAD_VERSION}\; "
"${CONFIG_IDF_TARGET}\; ${OT_BUILD_TIMESTAMP}")
idf_component_register(SRC_DIRS "${src_dirs}"
EXCLUDE_SRCS "${exclude_srcs}"
INCLUDE_DIRS "${public_include_dirs}"That's the only change I've needed to make to ensure a successful compilation without |
|
Yes - sure - take your time. Non-GIT build support was in the queue for so long time, that even if it needs a couple of extra weeks, no big deal I guess. I suggest you just mark one or both of your PRs as "Draft" until you feel they are ready for submission and review, and then you can un-"draft" them. |
|
I got pretty caught out by failures that were difficult to reproduce after the first successful build yesterday, so I reconsidered my approach to testing and made it a lot more repeatable. I'm confident after thorough testing that this change is doing the right thing, and I've included annotated Interestingly enough, I did run into the same issue with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When trying following in NixOS, it fails because this crate expects
.gitto exist for$ESP_IDF: https://github.com/knarkzel/hello-esp32c3The text was updated successfully, but these errors were encountered: