Skip to content

Conversation

facchinm
Copy link
Member

Test PR for #235

@ArduinoBot
Copy link
Contributor

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-308.zip

ℹ️ To test this build:

  1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

@matthijskooijman
Copy link
Collaborator

I had a look at this, I found that the core build path (and thus the core archive path) is made absolute here: https://github.com/arduino/arduino-builder/blob/master/add_additional_entries_to_context.go#L44

If the buildpath would be made absolute earlier, than all these filepath.Abs() could be removed, I guess.

If the path is made absolute, this is probably the best place: https://github.com/arduino/arduino-builder/blob/master/arduino-builder/main.go#L314

If no explicit build path is generated, an automatic one is generated which is probably already absolute: https://github.com/arduino/arduino-builder/blob/master/generate_buildpath_if_missing.go
https://github.com/arduino/arduino-builder/blob/master/generate_buildpath_if_missing.go

@facchinm
Copy link
Member Author

Agree with going back to the source

ctx.BuildPath = buildPath
🙂 Will modify the PR in a bit-

@facchinm facchinm force-pushed the relative_build_path branch from b9d154d to ce2bb0b Compare November 22, 2018 13:24
@ArduinoBot
Copy link
Contributor

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-308.zip

ℹ️ To test this build:

  1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

@facchinm facchinm added this to the next milestone Nov 23, 2018
@facchinm facchinm changed the title Pass absolute build path to filepath.Rel Make build path absolute in whole context Nov 23, 2018
@facchinm facchinm merged commit 8e500a9 into arduino:master Dec 19, 2018
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