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.

Sign up for GitHub

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

Jump to bottom

Dependency tree #29

Closed
wants to merge 7 commits into from
Closed

Dependency tree #29

wants to merge 7 commits into from

Conversation

kobbled
Copy link
Contributor

@kobbled kobbled commented Apr 11, 2019

This is my proposed solution to issue #28

kobbled added 6 commits April 10, 2019 23:07
@kobbled
logger.warn is depreciated to warning
320f8c3
@kobbled
add graph class
e641137 
creates a dependency graph stored as an object of tuples named packages. See function 'graph_tests' for details on usage. Insource is to designate packages in and outside of source repositories.
@kobbled
remove duplicates in package lists
dd27ebf 
-keep package lists unique.
-check_pkg_dependencies was removed as it will be handled by the graph class.
@kobbled
create dependency graph
5f61836 
-remove check_pkg_dependencies
-build out the dependency graph recursively
-print dependency trees for all packages in source space
-filter out packages that will not be used.
-remove resolve_dependencies as it will be handled by create_dependency_graph
@kobbled
Fix circular dependency issue while resolving includes
eb40624 
-used a visted set to manage circular dependencies
@kobbled
provide option to build all objects instead of just source objects
39c5aab
@kobbled
Copy link
Contributor Author

kobbled commented Apr 11, 2019 •
edited by gavanderhoorn
Loading

This is the output I get using this branch for https://github.com/kobbled/rossum-test:

> rossum ../ -v -w
INFO     | This is rossum v0.1.7
DEBUG    | Checking: c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\build\robot.ini
WARNING  | No robot.ini in CWD, and no alternative provided, trying source space
DEBUG    | Checking: c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src\robot.ini
INFO     | Found robot.ini in source space
WARNING  | Found robot.ini contains potentially conflicting ktrans settings!
INFO     | Found Roboguide version: V9.1098 (Rev.F)
DEBUG    | Most likely FANUC base-dir: C:\Program Files (x86)\FANUC
INFO     | Using C:\Program Files (x86)\FANUC as FANUC software base directory
DEBUG    | Trying to auto-detect ktrans location ..
DEBUG    | Looking in C:\Program Files (x86)\FANUC\WinOLPC\bin ..
DEBUG    | Found ktrans.exe in C:\Program Files (x86)\FANUC\WinOLPC\bin
INFO     | ktrans location: C:\Program Files (x86)\FANUC\WinOLPC\bin\ktrans.exe
INFO     | Setting default system core version to: V9.10-1
DEBUG    | Trying to find support dir for core version: V9.10-1
DEBUG    | Looking in C:\Program Files (x86)\FANUC\WinOLPC\Versions\V910-1\support ..
DEBUG    | Found V9.10-1 support dir: C:\Program Files (x86)\FANUC\WinOLPC\Versions\V910-1\support
INFO     | Karel core support dir: C:\Program Files (x86)\FANUC\WinOLPC\Versions\V910-1\support
INFO     | ktransw location: ktransw.cmd
DEBUG    | Using build file template: C:\Users\matt\Dropbox\Programming\Fanuc\rossum-0.1.4-distrib\build.ninja.em
INFO     | Source space(s) searched for packages (in order: src, args):
INFO     |   c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src
INFO     |   c:\Users\matt\Dropbox\Programming\Git\rossum-test
DEBUG    | Searching in c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src
DEBUG    |   found 1 manifest(s)
DEBUG    | Searching in c:\Users\matt\Dropbox\Programming\Git\rossum-test
DEBUG    |   found 4 manifest(s)
DEBUG    | Found 5 manifest(s) total
DEBUG    | Loaded package.json from c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src
DEBUG    | Loaded package.json from c:\Users\matt\Dropbox\Programming\Git\rossum-test\errors\src
DEBUG    | Loaded package.json from c:\Users\matt\Dropbox\Programming\Git\rossum-test\Hash\src
DEBUG    | Loaded package.json from c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src
DEBUG    | Loaded package.json from c:\Users\matt\Dropbox\Programming\Git\rossum-test\Strings\src
INFO     | Found 4 package(s) in source space(s):
INFO     |   KUnit (v1.0.0)
INFO     |   errors (v0.0.1)
INFO     |   Hash (v0.0.1)
INFO     |   Strings (v0.0.1)
DEBUG    | Resolving dependencies for: KUnit, errors, Hash, Strings
DEBUG    |   KUnit:
DEBUG    |     Strings: found
DEBUG    |   Strings:
DEBUG    |     KUnit: found
DEBUG    |     errors: found
DEBUG    |   errors:
DEBUG    |   Hash:
DEBUG    |     KUnit: found
DEBUG    |   KUnit:
DEBUG    |     Strings: found
DEBUG    |   Strings:
DEBUG    |     KUnit: found
DEBUG    |     errors: found
DEBUG    |     Strings: found
DEBUG    |   Strings:
DEBUG    |     KUnit: found
DEBUG    |   KUnit:
DEBUG    |     Strings: found
DEBUG    |     errors: found
DEBUG    | Printing dependency tree for: KUnit
DEBUG    |   (KUnit) 1.0.0 *
DEBUG    |   |-- (Strings) 0.0.1
DEBUG    |   |   |-- (errors) 0.0.1
DEBUG    | Printing dependency tree for: errors
DEBUG    |   (errors) 0.0.1 *
DEBUG    | Printing dependency tree for: Hash
DEBUG    |   (Hash) 0.0.1 *
DEBUG    |   |-- (KUnit) 1.0.0
DEBUG    |   |   |-- (Strings) 0.0.1
DEBUG    |   |   |   |-- (KUnit) 1.0.0
DEBUG    |   |   |   |-- (errors) 0.0.1
DEBUG    | Printing dependency tree for: Strings
DEBUG    |   (Strings) 0.0.1 *
DEBUG    |   |-- (KUnit) 1.0.0
DEBUG    |   |-- (errors) 0.0.1
DEBUG    | Resolving includes for: KUnit, Strings, errors, Hash
DEBUG    |   KUnit
DEBUG    |     added 3 path(s)
DEBUG    |   Strings
DEBUG    |     added 3 path(s)
DEBUG    |   errors
DEBUG    |     added 1 path(s)
DEBUG    |   Hash
DEBUG    |     added 4 path(s)
DEBUG    | Generating src to obj mappings for: KUnit, errors, Hash, Strings
DEBUG    |   KUnit
DEBUG    |     adding: src\kunit.kl -) kunit.pc
DEBUG    |     adding: src\test\test_kunit.kl -) test_kunit.pc
DEBUG    |   errors
DEBUG    |     adding: src\errors.kl -) errors.pc
DEBUG    |   Hash
DEBUG    |     adding: src\hash.kl -) hash.pc
DEBUG    |     adding: src\test\test_hash.kl -) test_hash.pc
DEBUG    |   Strings
DEBUG    |     adding: src\strings.kl -) strings.pc
DEBUG    |     adding: src\test\test_strings.kl -) test_strings.pc
INFO     | Building 4 package(s)
INFO     | Build configuration:
INFO     |   source dir: c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src
INFO     |   build dir : c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\build
INFO     |   robot.ini : c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\src\robot.ini
INFO     | Writing generated rules to c:\Users\matt\Dropbox\Programming\Git\rossum-test\KUnit\build\build.ninja
DEBUG    | Processing template
DEBUG    | Shutting down empy
INFO     | Configuration successful, you may now run 'ninja' in the build directory.

@gavanderhoorn
Copy link
Owner

Thanks for the PR.

I haven't checked the code in any detail yet (I will do so later), but I'm curious: in order to avoid infinite recursion, would keeping a "visited"-list and breaking the recursion (when the dependency resolver encounters a dependency it's already visited) achieve the same result without changing to a tree structure?

This is not a 👎 for this PR. I'm just trying to make sure I understand the issue you describe in #28 and the solution your propose here.

@gavanderhoorn
Copy link
Owner

Could you contribute 320f8c3 in a separate PR? :)

@kobbled
Copy link
Contributor Author

kobbled commented Apr 23, 2019

I was wondering that myself as I have rewritten this a few times now trying to locate the problem, which led me down the depth first search algorithm and why the PR seems verbose. I will try and check this in the next few days, and maybe submit some separate pull requests along with 320f8c3.

@kobbled
Copy link
Contributor Author

kobbled commented Apr 23, 2019

Things I was trying to achieve was reduce redundancies in searches, such as searching through manifest files of packages that are not related to the source directory. It is not shown in the output, but for extra external paths it will remove packages not related to the source packages. Definitely haven't seen performance problems, I am just thinking what the debug output for 100 packages would be like.
Then I added a thing to print dependency trees in the debug, which I have seen in other kinds of build packages that I feel is a nice thing to have.

@kobbled
Copy link
Contributor Author

kobbled commented Apr 24, 2019

I'm closing this PR. Modifying only resolve_includes(), and resolve_includes_for_pkg() seems to have solved the issue. 320f8c3 is a warning for python3 it might conflict with python2 instances. I will open an issue to see if you want to change it at all.

@kobbled kobbled closed this Apr 24, 2019
@kobbled kobbled mentioned this pull request Apr 24, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kobbled @gavanderhoorn