Skip to content

Replace "unzip/tar" in build.xml by Ant's provided unzip/untar method #8682

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

Merged
merged 3 commits into from
Apr 18, 2019

Conversation

Pieter12345
Copy link
Contributor

Replace the "unzip" and "tar" command used while building Arduino by the unzip and untar method that Ant provides.
This fixes #8617, meaning that "ant run" will work again and no files/directories are created with weird permissions and share names anymore.

@facchinm
Copy link
Member

This PR looks awesome! I'm giving it a spin in jenkins to make sure it doesn't hurt our normal CI environment and then I'd be super happy to merge it

@facchinm facchinm added this to the Release 1.8.10 milestone Mar 20, 2019
@facchinm facchinm added the feature request A request to make an enhancement (not a bug fix) label Mar 20, 2019
@arduino arduino deleted a comment from ArduinoBot Mar 20, 2019
@arduino arduino deleted a comment from ArduinoBot Mar 20, 2019
@arduino arduino deleted a comment from ArduinoBot Mar 20, 2019
@MeAmAnUsername
Copy link

Works for me

(Windows 10, tested with git bash and cygwin)

@arduino arduino deleted a comment from ArduinoBot Mar 20, 2019
@Pieter12345
Copy link
Contributor Author

@facchinm Thanks for adding Unix/Linux support. Is there anything else in Arduino still using the zip/unzip from Cygwin? Otherwise they could be removed from the Arduino building guide once this is merged.

@Pieter12345
Copy link
Contributor Author

Pieter12345 commented Mar 21, 2019

Before these changes, I were not able to build using Windows command prompt and I had to use Cygwin due to "unzip" from the Cygwin install not being on my classpath. With these changes, I am able to build using Windows command prompt. However, interestingly enough there is a difference between building with the 2: A build from command prompt leaves the Adafruit_CircuitPlaygroups-1.8.1 and arduino-2.5.8 directories in the windows/work/libraries directory. Both directories contain only a .gitignore file. These directories cause the following warnings in the IDE later on:

Invalid library found in C:\Users\....\Arduino\build\windows\work\libraries\Adafruit_CircuitPlayground-1.8.1: no headers files (.h) found in C:\Users\....\Arduino\build\windows\work\libraries\Adafruit_CircuitPlayground-1.8.1
Invalid library found in C:\Users\....\Arduino\build\windows\work\libraries\arduino-2.5.8: no headers files (.h) found in C:\Users\....\Arduino\build\windows\work\libraries\arduino-2.5.8

Difference in build feedback:
In Cygwin, the [move] feedback is not there, whereas it is in Windows command prompt.

unzip:
     [echo] Unzipping Adafruit_Circuit_Playground-1.8.1.zip into folder windows/work/libraries
    [unzip] Expanding: C:\Users\....\Arduino\build\Adafruit_Circuit_Playground-1.8.1.
zip into C:\Users\....\Arduino\build\windows\work\libraries
     [move] Moving 122 files to C:\Users\....\Arduino\build\windows\work\libraries

This move is triggered by line 278 in build.xml:
<move file="${target.path}/libraries/@{name}-@{version}" tofile="${target.path}/libraries/@{foldername}"/>

I am unsure why this move task executes differently depending on the command prompt, but I can imagine that it has to do something with the fact that both directories are pulled from a different repos than the others and that those repos contain .gitignore files.

I will explore options and see if I can come up with something that makes both terminals have the same build result.

@Pieter12345
Copy link
Contributor Author

The issue mentioned above was caused by Ant's default excludes for directory tasks. Removing all default excludes is the proper way to resolve this problem since running "ant build/dist" using Cygwin on Windows apparentally already ignores these default excludes.

@facchinm
Copy link
Member

There's a problem on Linux (probably OSX too) about ant untar... It loses permissions (see https://stackoverflow.com/questions/1517297/how-can-i-use-the-ant-tar-task-and-preserve-file-permissions) . I'd revert unix systems into using untar and unzip binaries, even if it leads to more rules to be maintained.

@facchinm facchinm self-requested a review March 21, 2019 10:06
Copy link
Member

@facchinm facchinm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to revert unix commits and continue using untar and unzip utilities (not sure about unzip, untar for sure)

@Pieter12345
Copy link
Contributor Author

@facchinm Can you make these changes? I do not have a Unix environment on which I can test.

@facchinm
Copy link
Member

Sure, as soon as I get some "spare" time I'm on it 😉

@facchinm
Copy link
Member

@ArduinoBot build this please

@arduino arduino deleted a comment from ArduinoBot Mar 27, 2019
Pieter12345 and others added 3 commits March 29, 2019 02:55
Replace the "unzip" and "tar" command used while building Arduino by the unzip and untar method that Ant provides.
This fixes arduino#8617, meaning that "ant run" will work again and no files/directories are created with weird permissions and share names anymore.
Besides that these default excludes get ignored when using the Cygwin terminal on Windows, they cause Ant move tasks to not remove the 'moved' directory if it contains a `.gitignore`  or other default exclude file.
To overcome ant untar behaviour (executable bits being lost)
@Pieter12345
Copy link
Contributor Author

Rebased this PR onto master.

@facchinm facchinm merged commit eb48db2 into arduino:master Apr 18, 2019
@facchinm facchinm mentioned this pull request Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arduino IDE Crashed Soon After "ant run"(WIN7)
4 participants