Skip to content

Allow upload using binaries from build-path #675

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 9 commits into from

Conversation

cmaglie
Copy link
Member

@cmaglie cmaglie commented Apr 24, 2020

Please check if the PR fulfills these requirements

  • What kind of change does this PR introduce?

New flags are added to the upload command:

  -n, --upload-from-build-path   Optional, use binaries produced in build path for uploading.
      --build-path string        Optional, specify build path (can be used with --upload-from-build-path).

With -n, --upload-from-build-path the CLI will upload binaries from the build path.
--build-path force a custom build-path instead of the build-path autogenerated by the CLI.

The new behaviour is now the default if chained compile + upload (compile -u) is used.

  • What is the current behavior?
    Currently:
  1. arduino-cli compile -u ... will produce the binaries in the sketch folder will use them for the upload.
  2. arduino-cli compile -n -u ... will not work (because the -n/--dry-run will not copy binaries in the sketch folder and consequently the upload will fail.
  • What is the new behavior?
    After the patch
  1. arduino-cli compile -u ... will produce the binaries in the sketch folder but for the upload the binaries in the build path will be used for the upload. This should fix Upload build path is different than output build path #641 at least for the chained build+upload.
  2. arduino-cli compile -n -u ... will now work, it will upload without creating the binaries in the sketch folder (because the -n/--dry-run will not copy binaries in the sketch folder but the upload will use the binaries from the build-path 👍 ).
  • Does this PR introduce a breaking change?
    It should not.

@cmaglie cmaglie self-assigned this Apr 24, 2020
@cmaglie cmaglie requested a review from rsora April 24, 2020 12:10
@cmaglie cmaglie added this to the 0.11.0 milestone Apr 24, 2020
@cmaglie cmaglie changed the title Upload fix Allow upload using binaries from build-path Apr 24, 2020
@cmaglie cmaglie marked this pull request as draft April 24, 2020 15:52
@rsora
Copy link
Contributor

rsora commented Apr 24, 2020

After going through your code and a bit of discussion with @cmaglie here my considerations:

TL;DR: with #655 nad this PR, we are basically trying to solve a "Core configuration" problem, rather than a CLI problem

This is our context:

  • The CLI upload functionality is built upon the so called Java IDE "Export" functionality, that copies artifact to upload on the board in the sketch path
  • The Java IDE instead, during the upload phase, uses the so called "sketch build path" as the place for upload artifact searching, and does not use the export path for upload purposes
    Being the "Export" functionality a secondary Java IDE feature and not well documented at cores specification level, we a percentage of community cores (~20%) that do not implement correctly the "Export" (that means: mismatch between upload recipe file extension and export recipe file extension in most cases)

So, it seems that we have the following strategies to solve this issue in the CLI:

  1. Ask to community cores to fix their export misconfiguration (and hope we get all the fixes merged in a reasonable amount of time) without changing the CLI behavior
  2. Solve the problem merging this PR, maintaining the "Export" folder as the default upload artifact path, and giving the user flags for search in the build path for artifacts
  3. Change the default CLI behavior, using the "build path" folder as the default upload artifact path. We can provide a --export flag for the compile to explicitly call for it
  4. Ignore the "Export" configuration completely, and by default export all build artifacts generated in the build folder as <sketch_name>.ino.* without changing CLI behavior

Actually I believe that 4 could be the best strategy because:

  • In 2-step compile + upload scenario, even if the compile uses different build path via flag, the upload command is not affected, as every artifact will be "exported" regardless of the core configuration
  • In a combo compile --upload scenario ther is no need to pass anything to the "upload" invocation
  • after having the CLI completely merged in the Java IDE, we can ask to community to "clean" their cores from export configurations, without worrying too much if te PRs will be merged in time or not.

@cmaglie WDYT?

@cmaglie cmaglie mentioned this pull request May 6, 2020
4 tasks
@cmaglie cmaglie closed this in #687 May 15, 2020
@cmaglie cmaglie deleted the upload-fix branch May 19, 2020 15:28
@per1234 per1234 added type: enhancement Proposed improvement topic: code Related to content of the project itself labels Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upload build path is different than output build path
3 participants