Refactor scan resources to account for base_paths #3560

theotherjimmy · 2017-01-10T19:15:54Z

Description

Separate the logic for managing the ignores lists from the scan resources calls. Improves the speed of the scan by ignoring all subdirectories of an ignored directory instead of just the files. Allows ignores to be specific to a basepath. This is useful to the online compiler as it can change the basepath to allow ignores to affect imported libraries despite them not being a subdirectory of the project.

Status

READY

Todos

Tests
- /morph test
- online ecosystem

theotherjimmy · 2017-01-10T19:16:13Z

/morph test

bridadan

Just a doc request, everything else looks ok.

bridadan · 2017-01-10T20:17:47Z

tools/toolchains/__init__.py

@@ -512,6 +512,10 @@ def is_ignored(self, file_path):
                return True
        return False

+    def add_ignore_patterns(self, root, base_path, patterns):


Would you mind adding a doc string to this? root and base_path are a bit confusing.

mbed-bot · 2017-01-10T20:19:40Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1376

Example Build failed!

bridadan · 2017-01-10T20:19:54Z

tools/toolchains/__init__.py

@@ -577,7 +582,7 @@ def _add_dir(self, path, resources, base_path, exclude_paths=None):
                    # Ignore toolchain that do not match the current TOOLCHAIN
                    (d.startswith('TOOLCHAIN_') and d[10:] not in labels['TOOLCHAIN']) or
                    # Ignore .mbedignore files
-                    self.is_ignored(join(dir_path,"")) or
+                    self.is_ignored(join(relpath(root, base_path), d,"")) or


I realize this was here before, but do you know why the path is being joined with ""?

To get that extra "/" on the end.

theotherjimmy · 2017-01-10T20:37:49Z

The failure looks unrelated to the codebase; it looks like a clone failure or something like that.

/morph test

mbed-bot · 2017-01-10T21:46:42Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1377

All builds and test passed!

bridadan

One last nit!

bridadan · 2017-01-10T22:28:44Z

tools/toolchains/__init__.py

+        """Add a series of patterns to the ignored paths
+
+        Positional arguments:
+        root - the path too the ignore location


too? 😄

lol. fixing

theotherjimmy · 2017-01-11T20:56:13Z

/morph test

bridadan

LGTM

mbed-bot · 2017-01-11T22:05:33Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1382

All builds and test passed!

0xc0170 · 2017-01-12T12:03:12Z

Please add more info to the commit msg if possible. In this case, description provided in the first message could be in the first commit message.

Apart from that, LGTM

0xc0170 · 2017-01-12T14:55:22Z

Waiting for CI to be green

theotherjimmy · 2017-01-12T20:27:41Z

@jupe Why does the client-testapp build fail with Replace the file source/identity_dev_security.c with valid developer certificate? That seems like something that the CI should not encounter.

jupe · 2017-01-13T08:20:53Z

17:07:56 [K64F ARM] [Error] identity_dev_security_systest.c@38,0:  #35: #error directive: "Replace the file source/identity_dev_security.c with valid developer certificate"
17:07:56 
[K64F ARM] [ERROR] "./configs/identity_dev_security_systest.c", line 38: Error:  #35: #error directive: "Replace the file source/identity_dev_security.c with valid developer certificate"
17:07:56 [K64F ARM] ./configs/identity_dev_security_systest.c: 0 warnings, 1 error
17:07:56 [K64F ARM] 
17:07:56 [K64F ARM] [mbed] ERROR: "python" returned error code 1.
17:07:56 [K64F ARM] [mbed] ERROR: Command "python -u /builds/ws/ARMmbed/mbed-client-testapp/master/mbed-client-testapp/mbed-os/tools/make.py -t ARM -m K64F --source . --build ./BUILD/K64F/ARM -c" in "/builds/ws/ARMmbed/mbed-client-testapp/master/mbed-client-testapp"

@teetak01 can you help with this ?

teetak01 · 2017-01-13T08:39:38Z

For some reason mbed-cli is not respecting the .mbedignore. Is the build-environment somehow special?

jupe · 2017-01-13T08:42:57Z

true, there is mbedignore file with configs/identity_dev_security_systest.c but still it fails.. Smells bug in this PR.

teetak01 · 2017-01-13T08:58:18Z

This rule is no longer in effect with this PR: https://github.com/ARMmbed/mbed-client-testapp/blob/master/.mbedignore#L10

bridadan · 2017-01-13T17:56:01Z

@theotherjimmy Mind taking a look?

theotherjimmy · 2017-01-13T17:56:36Z

@bridadan I'm already looking.

theotherjimmy · 2017-01-13T18:02:33Z

Sorry guys. I apparently started with the wrong commit. I have updated that commit to the correct one via rebasing. It should work now

/morph test

theotherjimmy · 2017-01-13T19:14:40Z

Cam-CI looks like a Java Error.

mbed-bot · 2017-01-13T21:29:11Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1398

All builds and test passed!

theotherjimmy · 2017-01-13T22:03:16Z

I restarted Cam-CI. Still a Java error.

bridadan · 2017-01-17T15:29:12Z

retest uvisor

bridadan · 2017-01-17T20:02:20Z

/morph export-build

mbed-bot · 2017-01-17T21:26:32Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph export-build

Output

mbed Build Number: 88

All exports and builds passed!

bridadan · 2017-01-17T21:52:53Z

Looks good to me, final OK is up to you @0xc0170

theotherjimmy added the needs: CI label Jan 10, 2017

bridadan reviewed Jan 10, 2017

View reviewed changes

theotherjimmy force-pushed the refactor-ignores branch from b203035 to 5c786c6 Compare January 11, 2017 20:55

bridadan approved these changes Jan 11, 2017

View reviewed changes

bridadan added ready for merge and removed needs: CI labels Jan 11, 2017

0xc0170 added needs: CI and removed ready for merge labels Jan 12, 2017

theotherjimmy added 2 commits January 13, 2017 12:01

Refactor scan resources to account for base_paths

39a5caa

Add docstrings

c14a5b9

theotherjimmy force-pushed the refactor-ignores branch from 5c786c6 to c14a5b9 Compare January 13, 2017 18:01

bridadan added needs: review and removed needs: CI labels Jan 17, 2017

0xc0170 approved these changes Jan 19, 2017

View reviewed changes

0xc0170 added ready for merge release-version: 5.3.4 and removed needs: review ready for merge labels Jan 19, 2017

0xc0170 merged commit 0ea544f into ARMmbed:master Jan 26, 2017

adbridge added release-version: 5.3.4 and removed release-version: 5.3.4 labels Jan 27, 2017

Refactor scan resources to account for base_paths #3560

Refactor scan resources to account for base_paths #3560

Uh oh!

Conversation

theotherjimmy commented Jan 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Status

Todos

Uh oh!

theotherjimmy commented Jan 10, 2017

Uh oh!

bridadan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbed-bot commented Jan 10, 2017

Output

Uh oh!

bridadan Jan 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Jan 10, 2017

Uh oh!

mbed-bot commented Jan 10, 2017

Output

Uh oh!

bridadan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Jan 11, 2017

Uh oh!

bridadan left a comment

Choose a reason for hiding this comment

Uh oh!

mbed-bot commented Jan 11, 2017

Output

Uh oh!

0xc0170 commented Jan 12, 2017

Uh oh!

0xc0170 commented Jan 12, 2017

Uh oh!

theotherjimmy commented Jan 12, 2017

Uh oh!

jupe commented Jan 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teetak01 commented Jan 13, 2017

Uh oh!

jupe commented Jan 13, 2017

Uh oh!

teetak01 commented Jan 13, 2017

Uh oh!

bridadan commented Jan 13, 2017

Uh oh!

theotherjimmy commented Jan 13, 2017

Uh oh!

theotherjimmy commented Jan 13, 2017

Uh oh!

theotherjimmy commented Jan 13, 2017

Uh oh!

mbed-bot commented Jan 13, 2017

Output

Uh oh!

theotherjimmy commented Jan 13, 2017

Uh oh!

bridadan commented Jan 17, 2017

Uh oh!

bridadan commented Jan 17, 2017

Uh oh!

theotherjimmy commented Jan 10, 2017 •

edited

Loading

bridadan Jan 10, 2017 •

edited

Loading

jupe commented Jan 13, 2017 •

edited

Loading