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

Fix for projects exported as a zip file (affects online compiler) #9967

Merged
merged 8 commits into from Apr 9, 2019

Conversation

Projects
None yet
7 participants
@bridadan
Copy link
Contributor

commented Mar 6, 2019

Description

This is a follow up to #9947 in an attempt to fix #8411.

Currently depends on #9995 and #9964.

NOTE I do not have a vscode environment to test these changes, however I can see the .vscode directory is now being included correctly.

@janjongboom may have an interest in testing this as well.

To test this, you'll need to run the following command in an mbed-os project:

mbed export -m <target> -i vscode_gcc_arm -z

(The -z is so the project will be zipped up)

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

@naveenkaje
@theotherjimmy (there was also an issue when Makefiles were zipped up, perhaps you can double check my resources change)

Release Notes

@ciarmcom ciarmcom requested review from naveenkaje, theotherjimmy and ARMmbed/mbed-os-maintainers Mar 7, 2019

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Mar 7, 2019

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2019

@bridadan Unless I'm mistaken, I think that the conditional exporting was a good thing, at least offline.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

@theotherjimmy was the idea there that you wouldn't blow away your new configuration if you re-export?

The trouble there is if you re-zip and the files are present, then they will not be re-generated and therefore not re-zipped. Perhaps if you supply "export to a zip", it can always re-generate or something?

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 7, 2019

Oh dear I seem to have broken many a tools test.

@bridadan bridadan force-pushed the bridadan:fix_vscode_makefile_zip branch Mar 7, 2019

parent_name_parts = components[:n]
if into_path:
parent_name_parts.insert(0, into_path)
parent_name = self._sep.join(parent_name_parts)

This comment has been minimized.

Copy link
@bridadan

bridadan Mar 7, 2019

Author Contributor

@theotherjimmy I believe this is the correct fix (see my commit message). If you have any concerns about this let me know.

This comment has been minimized.

Copy link
@theotherjimmy

theotherjimmy Mar 8, 2019

Contributor

You're correct. That's the fix.

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

Also added dependency on #9995, though its not critical for this PR, just for my own testing of this PR. If ed2c2f5 needs to be kicked out just let me know.

@bridadan bridadan force-pushed the bridadan:fix_vscode_makefile_zip branch to a86b4b9 Mar 8, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2019

@theotherjimmy I've removed the commit that enabled unconditional generation of vscode files to ensure we don't wipe out any user-made changes. If we'd like to revisit that behavior in the future, we should probably do it for all exporters at the same time.

@ARMmbed/mbed-os-maintainers I'm done with my changes pending any other comments.

@@ -29,6 +29,7 @@
from copy import copy
from yaml import dump_all
import argparse
from past.builtins import basestring

This comment has been minimized.

Copy link
@naveenkaje

naveenkaje Mar 11, 2019

Contributor

is basestring being used? if so, please ignore this comment

This comment has been minimized.

Copy link
@bridadan

bridadan Mar 11, 2019

Author Contributor

I can't ignore your comments 😄 Yeah its used later in the file.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

Note: Still waiting on #9995

@cmonr cmonr added needs: work and removed needs: review labels Mar 27, 2019

bridadan added some commits Mar 7, 2019

Move all generated file paths to FileRefs in the exporters.
The FileRefs allow you to preserve the correct file paths in the online
compiler. It also allows you to preserve the correct file paths for
generated files.
Create template for missing generated vscode file
This file was being dumped to the filesystem without going through the
"gen_file" mechanism, thus it was missed when being zipped up.
Fixing zipped makefile exports.
When zipping up projects, the makefile exporter brings every directory
supplied as --source under the same directory, even if they are in a
parent directory. There was some code that was clearing the leading
"../" components. This lead to an empty string ("") being supplied to
the "into_path" arg for "resources.add_directory". Since "" is not None,
the default behavior to place it in the same directory was not being
used. The extra "" caused a leading "/" to be added, making everything
placed a the absolute root of the filesystem ("/").

Now we check to see if the "into_path" is an empty string and ignore it
if that's the case.

@bridadan bridadan force-pushed the bridadan:fix_vscode_makefile_zip branch from a86b4b9 to 60910c0 Apr 8, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Rebased!

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Apr 8, 2019

Also #9995 was merged some time ago.

@bridadan bridadan changed the title Fix for vscode export when zipped Fix for projects exported as a zip file (affects online compiler) Apr 8, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Apr 9, 2019

CI started

@mbed-ci

This comment has been minimized.

Copy link

commented Apr 9, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170

0xc0170 approved these changes Apr 9, 2019

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Apr 9, 2019

@cmonr cmonr merged commit fba8156 into ARMmbed:master Apr 9, 2019

26 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARM Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARM Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 9223 cycles (-788 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.