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

Decide tools installation directory #11

Closed
Tracked by #10
N3xed opened this issue Sep 10, 2021 · 5 comments
Closed
Tracked by #10

Decide tools installation directory #11

N3xed opened this issue Sep 10, 2021 · 5 comments

Comments

@N3xed
Copy link
Collaborator

N3xed commented Sep 10, 2021

  • pio uses the platformio default in ~/.platformio
  • Should both strategies use the same directory (e.g. <?>/.embuild/platformio for pio and <?>/.embuild/espressif for native)?
  • Where should the default directory be: home, workspace or out directory?

@ivmarkov already enumerated options with pros and cons:

That is to say, whatever we decide to be the default, it should be for both build systems. To summarize, perhaps 4 options:

  • Option 1 - install and use the default global locations (i.e. ~/.platformio and ~/.espressif)
    • Pros:
      • Does not pollute the binary crate with stuff that should be .gitignore-d
      • Downloaded toolchains etc. can be re-used across builds of different binary crates
      • Already downloaded toolchains etc. will just be used
    • Cons:
      • Patching ESP-IDF by the build itself is not so ideal as this is kinda build-specific
      • Messes up with the global user configuration of either PlatformIO or Espressif idf.py
  • Option 2 - install and use an embuild-specific global location (i.e. ~/.embuild/platformio and ~/.embuild/espressif)
    • Pros:
      • Does not pollute the binary crate with stuff that should be .gitignore-d
      • Downloaded toolchains etc. can be re-used across builds of different binary crates
      • Already downloaded toolchains etc. will just be used
      • Does not mess up with the user configuration
    • Cons:
      • Patching ESP-IDF by the build itself is not so ideal as this is kinda build-specific
      • Creates something outside the binary crate directory
  • Option 3 - install and use an embuild-specific location within the root of the binary crate (i.e. .embuild/platformio and .embuild/espressif)
    • Pros:
      • Does not mess up with the user configuration
      • No global state
      • Patching ESP-IDF OK as it remains build specific
    • Cons:
      • Pollutes the binary crate with stuff that should be .gitignore-d
      • Still does not follow the cargo policy that everything should be in the $OUT_DIR
      • Downloaded toolchains etc. cannot be re-used across builds of different binary crates
  • Option 4 - install and use an embuild-specific location within the OUT_DIR of the binary crate (i.e. $OUT_DIR/embuild/platformio and $OUT_DIR/embuild/espressif)
    • Pros:
      • Does not mess up with the user configuration
      • No global state
      • Patching ESP-IDF OK as it remains build specific
      • Does not pollute the binary crate with stuff that should be .gitignore-d
      • Does not follow the cargo policy that everything should be int he $OUT_DIR
      • cargo clean means everything is removed, including ESP-IDf patches. Really clean state
    • Cons:
      • cargo clean means everything will be re-downloaded again
      • Downloaded toolchains etc. cannot be re-used across builds of different binary crates

Preferences for a default?
Mines are:

  • Option 2
  • Option 3 (if global state is something you guys feel uneasy)

In any case, having a flag/switch/env var/whatever so that the build framework is operating in Option 1 mode is a prerequisite IMO.

@N3xed N3xed changed the title Decide tools installation directory Decide tools installation directory for native Sep 10, 2021
@N3xed N3xed changed the title Decide tools installation directory for native Decide tools installation directory Sep 10, 2021
@N3xed
Copy link
Collaborator Author

N3xed commented Oct 17, 2021

Okay so I think what we decided on was:

For the native build:

  • Global install dir: ~/.espressif
  • Local install dir (used by default): <workspace-dir>/.embuild/espressif

For the pio build:

  • Continue to use the global install dir ~/.platformio, which is the pio default.

@igrr
Copy link

igrr commented Oct 17, 2021

I have missed the initial comment, so it might be a bit late, but this statement regarding "global" IDF install

Messes up with the global user configuration of either PlatformIO or Espressif idf.py

shouldn't be true; if it is, it's probably a bug. At least as far as tools installation (via idf_tools.py) goes. The tools are installed side by side and each IDF version picks the tools to use.

For IDF itself, it is typically not installed into ~/.espressif, and given that it's patched I agree it makes sense to install it into the local build directory.

@N3xed
Copy link
Collaborator Author

N3xed commented Oct 17, 2021

Well, installing the tools into ~/.espressif shouldn't cause issues, that's what I thought too (because this uses the exact same mechanism to install the tools as the esp-idf itself, as far as I know). But the problem was with installing the esp-idf source (at least for me). Currently, both the source and tools are always installed in the same directory. And when installing the source in the global location and patching it, that could cause problems, should the user installation of the esp-idf source and our installation coincide.

With #19, this problem still exists, as again, both the source and tools are always installed in the same location. Changing this would make it more complicated.
The default is to install both things locally, but the user can opt into the global install. Also, I think the goal is to eventually not have to patch the esp-idf. Additionally, unless a specific hash is installed, the esp-idf source folder always gets the tag or branch
name suffixed so this should also prevent collisions in most cases.

So with that, I think that this shouldn't really be an issue. Please tell me if you think otherwise.

For IDF itself, it is typically not installed into ~/.espressif

That's good to know. And also supports my conclusion 😄.

@ivmarkov
Copy link
Collaborator

ivmarkov commented Oct 21, 2021

Currently we are converging on the following (just to summarize the discussion, please correct me if I'm wrong on any of these):

  • Native builds:
    • By default, <binary crate>/.embuild/espressif will be picked up as an install location
    • User can override this with an environment variable, and use either a user-defined location, or the global ~/.espressif, which is the default one for stock ESP-IDF installations
    • In both cases, ESP-IDF will be installed in the "espressif" folder as defined above, however the user should have the option (i.e. an environment variable), to override this location. This is a bit odd when the "espressif" folder is the global one in the presence of patching, however the expectation is to not do patching in future (or use it by picking up a patched GIT fork of ESP-IDF)
  • PIO builds:
    • (Not yet the default, but will be soon): <binary crate>/.embuild/platformio will be picked up as an install location
    • User can override this with an environment variable, and use either a user-defined location, or the global ~/.platformio, which is the default one for stock PlatformIO installations
    • In both cases, ESP-IDF will be installed in the "platfromio/framework_espidf" folder as defined above. The user (due to PlatformIO restrictions) will NOT have the option to override this location. This is a bit odd when the "platformio" folder is the global one in the presence of patching, however the expectation is to not do patching in future (or use it by picking up a patched GIT fork of ESP-IDF)

@ivmarkov
Copy link
Collaborator

ivmarkov commented Dec 4, 2021

The PlatformIO aspect of this should now be fixed by 9022587

I'll publish a new release now, with these changes as well as the previous ones from @N3xed that unified the handling of sdkconfig* files and glob file copies.

@ivmarkov ivmarkov closed this as completed Dec 4, 2021
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

No branches or pull requests

3 participants