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

Assignment 2.1: Build Automation Tool #60

Closed
wants to merge 51 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
ca5e40c
Add Config Parser
ankurdubey521 Sep 9, 2019
a338501
Add Command Line Runner
ankurdubey521 Sep 9, 2019
42557d3
Implement Builder
ankurdubey521 Sep 9, 2019
1f28c4f
Add tests for Builder
ankurdubey521 Sep 9, 2019
f33e3f7
Fix tests and PEP8 violations
ankurdubey521 Sep 9, 2019
6270d50
Fix test and add docstrings
ankurdubey521 Sep 9, 2019
f0be0a3
General Syntax and Naming Fixes
ankurdubey521 Sep 10, 2019
1e5bd0d
Implement Command Class and add basic validation
ankurdubey521 Sep 10, 2019
cd94bc8
Fix Main.py
ankurdubey521 Sep 10, 2019
9bcb8cb
Add support for Relative Paths
ankurdubey521 Sep 10, 2019
c633b29
Add support for Root Referenced Commands
ankurdubey521 Sep 11, 2019
ac947b8
Implement Check for Circular Dependencies
ankurdubey521 Sep 11, 2019
d71d9c4
Move main function to it's own class
ankurdubey521 Sep 11, 2019
277b7d6
Pre-Generate all Command Objects while initializing BuildConfig Object
ankurdubey521 Oct 3, 2019
f1c42c3
Remove _hasFiles and _hasDependencies from BuildConfig
ankurdubey521 Oct 3, 2019
5f26c51
Print Logs when doing a dry run
ankurdubey521 Oct 3, 2019
22580e3
Move Builder Class to it's own file and Save root_dir_abs
ankurdubey521 Oct 3, 2019
a1b25bd
Move command runner to Builder Class
ankurdubey521 Oct 3, 2019
c0da0bc
Use unittest setup method instead of @classMethod
ankurdubey521 Oct 3, 2019
166c5db
Catch any exceptions that might be thrown in main.py by Builder
ankurdubey521 Oct 3, 2019
923ead1
Fixes in Builder Tests
ankurdubey521 Oct 3, 2019
5da1c4c
Add type hints
ankurdubey521 Oct 3, 2019
980e9e1
Switch to Argparse
ankurdubey521 Oct 3, 2019
c421aa6
Implement class for listening changes to files
ankurdubey521 Oct 3, 2019
eabddd1
Remove redundant for loop from file_watcher
ankurdubey521 Oct 3, 2019
7df0d4a
Integrate File watching in Build System
ankurdubey521 Oct 3, 2019
fc505ed
Make Root Directory Configurable
ankurdubey521 Oct 3, 2019
9c83a4b
Code Quality Fixes
ankurdubey521 Oct 5, 2019
4ba29bc
Add Topological Sort Code
ankurdubey521 Oct 7, 2019
0cf3e73
Implement execution of dependency rules in parallel
ankurdubey521 Oct 7, 2019
54a3e62
Cleanup
ankurdubey521 Oct 7, 2019
31226cc
Include file of dependencies when listening for changes. Also provide…
ankurdubey521 Oct 8, 2019
28a1c63
Document ParallelBuilder
ankurdubey521 Oct 8, 2019
60c414e
Document TopologicalSort
ankurdubey521 Oct 8, 2019
4e1cadf
Document buildconfig
ankurdubey521 Oct 8, 2019
86c116c
Add global constants
ankurdubey521 Oct 8, 2019
cfae026
Remove Redundant Exceptions
ankurdubey521 Oct 8, 2019
b77e20b
Run tests from Tempdir instead of running them from source
ankurdubey521 Oct 8, 2019
3658eb8
Remove Redundant path correction function from TestFileWatcher
ankurdubey521 Oct 8, 2019
ce7515e
Refactoring
ankurdubey521 Oct 9, 2019
2ec874f
Implement Logging
ankurdubey521 Oct 9, 2019
64f9272
Stop spawning more threads if build fails for dependency
ankurdubey521 Oct 9, 2019
18f8219
Move Test file contents to Testing Scripts
ankurdubey521 Oct 9, 2019
13b446d
Use os.path.join instead of manually joining paths
ankurdubey521 Oct 9, 2019
baf6e64
Optimize imports and use os.path.join in tests
ankurdubey521 Oct 10, 2019
f574672
Fix Logging
ankurdubey521 Oct 30, 2019
4f9bdfe
Refactoring
ankurdubey521 Oct 30, 2019
d9f55eb
Implement JSON Caching
ankurdubey521 Oct 30, 2019
1176bfa
Fix Tests
ankurdubey521 Oct 30, 2019
1f9d18b
Remove redundant paramaters from _execute_build_rule
ankurdubey521 Oct 30, 2019
f516ae0
Implement parallel execution using Popen polling
ankurdubey521 Oct 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions solutions/ankurdubey521/Build Automation Tool/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/venv/

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions solutions/ankurdubey521/Build Automation Tool/.idea/misc.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions solutions/ankurdubey521/Build Automation Tool/.idea/vcs.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file.
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import json


class BuildConfig:
"""Parses and Stores build.config"""
def __init__(self, json_path):
# Parse JSON
json_file = open(json_path)
self._raw_json = json.load(json_file)
json_file.close()
Copy link
Owner

Choose a reason for hiding this comment

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

You should use something like:

with open(json_path) as file_handle:
  _ = json.load(file_handle)


# Validate the config file
self.validate()

self._command = {}
Copy link
Owner

Choose a reason for hiding this comment

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

Proper naming is important:

  • _command suggests a single item.
  • _commands suggests a list of items.
  • I would use self.name_to_command because it looks like a map.

for command in self._raw_json:
self._command[command['name']] = command

def command_names(self):
"""Returns list of Commands"""
return list(self._command)

def deps(self, command_name):
"""Return List of Dependencies"""
return self._command[command_name]['deps']

def command(self, command_name):
"""Returns Command String"""
return self._command[command_name]['command']

def files(self, command_name):
"""Returns List of Associated Files"""
return self._command[command_name]['files']
Copy link
Owner

Choose a reason for hiding this comment

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

Create a Command class, and then move these getters there.
That will leave you with a single getter here: "get_command(name)"


def validate(self):
# TODO: Think about cases and Implement this
pass


if __name__ == "__main__":
pass
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import subprocess


def run(command_string, cwd, print_command):
"""Run Command and Return Exit Code. Optionally Print the command itself"""
if print_command:
print(command_string)
return subprocess.run(command_string, shell=True, cwd=cwd).returncode


if __name__ == '__main__':
Copy link
Owner

Choose a reason for hiding this comment

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

This section does nothing. Remove it.

pass
42 changes: 42 additions & 0 deletions solutions/ankurdubey521/Build Automation Tool/Builder/main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
from Builder.lib.commandrunner import run
from Builder.lib.buildconfig import BuildConfig
import sys
import os


def execute(command, containing_folder_path):
"""Parses JSON, Resolves Dependencies and Executes Command"""

# Parse build.json in current directory
config = BuildConfig(containing_folder_path + "/build.json")

# Parse Dependencies First
try:
deps = config.deps(command)
Copy link
Owner

Choose a reason for hiding this comment

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

What if I wanted to run "build algorithms/sort_bubble"?
Since that is not in the current directory, this code would not find it (even though you have the relative path).

dep_count = len(deps)
print("Executing {} dependencies for {} in {}...".format(dep_count, command, containing_folder_path))
for dep in deps:
try:
# Command in Child Folder
dep_containing_folder_path, dep_command = dep.rsplit('/', 1)
dep_containing_folder_path = containing_folder_path + "/" + dep_containing_folder_path
execute(dep_command, dep_containing_folder_path)
Copy link
Owner

Choose a reason for hiding this comment

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

You need to add checks for circular dependencies.

except ValueError:
Copy link
Owner

Choose a reason for hiding this comment

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

ValueError can be generated by so many things that I have no idea when this will happen. Maybe you want to use a more specific exception?

# Command in Same Folder
execute(dep, containing_folder_path)
except KeyError:
# No Dependencies
print("No dependencies found for {} in {}...".format(command, containing_folder_path))

# Execute Command after processing dependencies
print("Executing {} in {}".format(command, containing_folder_path))
return_value = run(config.command(command), containing_folder_path, print_command=True)

# Stop Execution if Command Fails
if return_value != 0:
exit(-1)


if __name__ == '__main__':
Copy link
Owner

Choose a reason for hiding this comment

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

It might still be useful to keep this main application logic in a separate file from your library.
I'd also recommend catching your exceptions and only showing the message, instead of the whole stack trace.

command = sys.argv[1]
execute(command, os.getcwd())
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[
{
"name": "clean",
"command": "rm -f *.o"
},
{
"name": "sort_bubble",
"files": ["sort_bubble.cpp"],
"command": "g++ -c sort_bubble.cpp"
},
{
"name": "sort_merge",
"files": ["sort_merge.cpp"],
"command": "g++ -c sort_merge.cpp"
},
{
"name": "sort_quick",
"files": ["sort_quick.cpp"],
"command": "g++ -c sort_quick.cpp"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include<vector>
#include<algorithm>

void sort_bubble(std::vector<int> &arr) {
sort(arr.begin(), arr.end());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include<vector>
#include<algorithm>

void sort_merge(std::vector<int> &arr) {
sort(arr.begin(), arr.end());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include<vector>
#include<algorithm>

void sort_quick(std::vector<int> &arr) {
sort(arr.begin(), arr.end());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[
{
"name": "clean",
"deps": ["algorithms/clean"],
"command": "rm -f test.o && rm -f test.out"
},
{
"name": "test",
"files": ["test.cpp"],
"command": "g++ -std=c++11 -c test.cpp"
},
{
"name": "run",
"deps": ["test", "algorithms/sort_bubble", "algorithms/sort_merge", "algorithms/sort_quick"],
"command": "g++ algorithms/sort_bubble.o algorithms/sort_merge.o algorithms/sort_quick.o test.o -o test.out && ./test.out"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#include<iostream>
#include<vector>

void sort_quick(std::vector<int>&);
void sort_merge(std::vector<int>&);
void sort_bubble(std::vector<int>&);

void print(const std::vector<int> &vec) {
for(auto &x: vec) {
std::cout << x << " ";
}
std::cout << "\n";
}

int main() {
std::vector<int> a = {5, 4, 3, 2, 1}, b = a, c = a;
sort_quick(a);
sort_merge(b);
sort_bubble(c);
print(a);
print(b);
print(c);
return 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"name": "clean",
"deps": ["algorithms/clean"],
"command": "rm -f test.o && rm -f test.exe"
},
{
"name": "test",
"files": ["test.cpp"],
"command": "g++ -std=c++11 -c test.cpp"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import unittest
from Builder.lib.buildconfig import BuildConfig


class TestJsonParser(unittest.TestCase):
def test_sample_json_parser_correctly(self):
json_path = 'json/test1.json'
Copy link
Owner

Choose a reason for hiding this comment

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

The input to your BuildConfig class should be a directory, not a specific file.
The file name "build.json" is fixed, so "test1.json" violates that pattern.

You can add the "build.json" suffix in the BuildConfig class, instead of relying on the caller to add it.

config = BuildConfig(json_path)
self.assertEqual(config.command_names(), ['clean', 'test'])
self.assertEqual(config.deps('clean'), ["algorithms/clean"])
self.assertEqual(config.command('clean'), "rm -f test.o && rm -f test.exe")
Copy link
Owner

Choose a reason for hiding this comment

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

Interesting, these utility methods can definitely be useful, but I'd recommend better naming here.
In this case, adding a "get_" prefix to all of them will make it clear that these are not "set_" or "has_" or "check_" or whatever else methods.



if __name__ == '__main__':
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import unittest
import subprocess
from Builder.main import execute


class TestBuilder(unittest.TestCase):
def test_compilation(self):
path = '/home/ankurdubey/Code/Github/project-omega/solutions' \
Copy link
Owner

Choose a reason for hiding this comment

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

This path is non-transferrable. If I downloaded this code in my machine, this test would fail, which is bad.

'/ankurdubey521/Build Automation Tool/Builder/tests/filetree'
execute('run', path)
exec_path = '"' + path + '/test.out' + '"'
result = subprocess.run(exec_path, shell=True, capture_output=True, text=True)
self.assertEqual(result.stdout, '1 2 3 4 5 \n1 2 3 4 5 \n1 2 3 4 5 \n')
execute('clean', path)


if __name__ == '__main__':
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import unittest
from Builder.lib.commandrunner import run


class TestCommandRunner(unittest.TestCase):
def test_basic_command(self):
command = "echo 'Hello World!'"
exit_code = run(command, print_command=False, cwd='/')
self.assertEqual(0, exit_code)

def test_nonzero_exit_code(self):
command = "exit 1"
exit_code = run(command, print_command=False, cwd='/')
Copy link
Owner

Choose a reason for hiding this comment

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

print_command=False

This should be a default argument value.

self.assertEqual(1, exit_code)


if __name__ == '__main__':
unittest.main()