Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.idea
venv
*.pyc
*.db
4 changes: 4 additions & 0 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
from flask_compress import Compress
from flask_sqlalchemy import SQLAlchemy
from flask_humanize import Humanize
import os.path
import sys

sys.path.append( os.path.dirname( os.path.abspath( __file__ ) ) )
Copy link
Owner

Choose a reason for hiding this comment

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

I never needed that so far. Is this to make the code robust against the working directory?

Copy link
Contributor Author

@igormironchik igormironchik Nov 29, 2017

Choose a reason for hiding this comment

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

Yes, this line is for create_db.py to be working when launched exactly from the root directory of the repository. I had problems with from config import ... Python couldn't find module config. And when I added this line to app/__init__.py create_db.py became working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, user can tune python's environment so all will work without this line. But I'm not sure about from config import ...., why not from app.config import ....?


app = Flask(__name__)
app.config.from_object('config')
Expand Down
54 changes: 29 additions & 25 deletions app/utils/Executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import subprocess
from threading import Timer, Thread
import psutil
from app.models import Patch, Run
from app.models import Patch, Run, File
import tempfile
import os
import datetime
Expand Down Expand Up @@ -74,33 +74,37 @@ def killer(process):

def workflow(self, patch: Patch):
assert self.__current_patch is None, 'no auto-concurrency!'
self.__current_patch = patch

# step 1: write patch to temp file
patchfile = tempfile.NamedTemporaryFile(delete=False, mode='w')
patchfile.write(patch.patch)
patchfile.close()

# step 2: apply patch
self.__execute_command_timeout('patch -p1 --input={patchfile}'.format(patchfile=patchfile.name), cwd='/')

# step 3: command pipeline
success = self.__apply_command(patch, 'build_command') and \
self.__apply_command(patch, 'quickcheck_command') and \
self.__apply_command(patch, 'test_command')

if success:
patch.state = 'survived'
db.session.commit()

# step 4: revert patch
self.__execute_command_timeout('patch -p1 --reverse --input={patchfile}'.format(patchfile=patchfile.name),

file = File.query.get(patch.file_id)

if file is not None:
self.__current_patch = patch

# step 1: write patch to temp file
patchfile = tempfile.NamedTemporaryFile(delete=False, mode='w')
patchfile.write(patch.patch)
patchfile.close()

# step 2: apply patch
self.__execute_command_timeout('patch -p1 --input={patchfile} {inputfile}'.format(patchfile=patchfile.name, inputfile=file.filename), cwd='/')

# step 3: command pipeline
success = self.__apply_command(patch, 'build_command') and \
self.__apply_command(patch, 'quickcheck_command') and \
self.__apply_command(patch, 'test_command')

if success:
patch.state = 'survived'
db.session.commit()

# step 4: revert patch
self.__execute_command_timeout('patch -p1 --reverse --input={patchfile} {inputfile}'.format(patchfile=patchfile.name, inputfile=file.filename),
cwd='/')
Copy link
Owner

Choose a reason for hiding this comment

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

What is the rationale for this changed block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed to work properly on Ubuntu 16.04. If launch patch -p1 --reverse --input={patchfile} then patch will fail and request file name to patch. And with patch -p1 --reverse --input={patchfile} {inputfile} all works as expected.


# step 6: delete patch file
os.remove(patchfile.name)
# step 6: delete patch file
os.remove(patchfile.name)

self.__current_patch = None
self.__current_patch = None

def __apply_command(self, patch: Patch, step: str):
print(patch, step)
Expand Down
8 changes: 4 additions & 4 deletions app/utils/SourceFile.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ def __create_patch(self, line_number: int, replacement: Replacement) -> str:
))

# lines: context before
patch_lines += [' ' + x + '\n' for x in context_before]
patch_lines += [' ' + x + "\r\n" for x in context_before]
Copy link
Owner

Choose a reason for hiding this comment

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

Why is \r\n needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Git can clone files with different line endings mode. By default, as I guess, git clone with \r\n and pushes with \n. So on my Ubuntu 16.04 I have repositories with \r\n line endings in files. So patch didn't work with \n and works with \r\n. But to be fair it's better to handle both situations...

# line: the old value
patch_lines.append('-' + old_line + '\n')
patch_lines.append('-' + old_line + "\r\n")
# line: the new value
if replacement.new_val is not None:
patch_lines.append('+' + new_line + '\n')
patch_lines.append('+' + new_line + "\r\n")
# lines: context after
patch_lines += [' ' + x + '\n' for x in context_after]
patch_lines += [' ' + x + "\r\n" for x in context_after]

patch_text = ''.join(patch_lines)
return patch_text
4 changes: 2 additions & 2 deletions db_create.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# coding=utf-8

from migrate.versioning import api
from config import SQLALCHEMY_DATABASE_URI
from config import SQLALCHEMY_MIGRATE_REPO
from app.config import SQLALCHEMY_DATABASE_URI
from app.config import SQLALCHEMY_MIGRATE_REPO
from app import db
Copy link
Owner

Choose a reason for hiding this comment

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

Wouldn't these changes also be required in db_migrate.py and db_upgrade.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. I didn't use these scripts, so didn't make any changes to them.

import os.path

Expand Down
4 changes: 2 additions & 2 deletions db_migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import imp
from migrate.versioning import api
from app import db
from config import SQLALCHEMY_DATABASE_URI
from config import SQLALCHEMY_MIGRATE_REPO
from app.config import SQLALCHEMY_DATABASE_URI
from app.config import SQLALCHEMY_MIGRATE_REPO

if __name__ == '__main__':
v = api.db_version(SQLALCHEMY_DATABASE_URI, SQLALCHEMY_MIGRATE_REPO)
Expand Down
4 changes: 2 additions & 2 deletions db_upgrade.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# coding=utf-8

from migrate.versioning import api
from config import SQLALCHEMY_DATABASE_URI
from config import SQLALCHEMY_MIGRATE_REPO
from app.config import SQLALCHEMY_DATABASE_URI
from app.config import SQLALCHEMY_MIGRATE_REPO

if __name__ == '__main__':
api.upgrade(SQLALCHEMY_DATABASE_URI, SQLALCHEMY_MIGRATE_REPO)
Expand Down