find_git_info propogates an error when ADDed file is not found in Dockerfile

[rnjudge]

Describe the bug
Using this Dockerfile I ran tern lock Dockerfile and the ADD command in the file broke Tern's run while it was trying to find the git information.

FROM photon:3.0

# install system dependencies
# photon:3.0 comes with toybox which conflicts with some dependencies needed for tern to work, so uninstalling
# toybox first
RUN tdnf remove -y toybox && tdnf install -y tar findutils attr util-linux python3 python3-pip python3-setuptools git

# install pip and tern
RUN pip3 install --upgrade pip && pip3 install tern

ADD Dockerfile /tmp

# make a mounting directory
RUN mkdir hostmount

ENTRYPOINT ["tern", "-q", "-b", "/hostmount"]
CMD ["-h"]

To Reproduce
Steps to reproduce the behavior:

1. Checkout tern from GitHub
2. Edit the Dockerfile in the tern directory to match the above file (hint: add the `ADD Dockerfile /tmp line)
3. See the error

Error in terminal

Traceback (most recent call last):
  File "/home/rjudge/ternenv/bin/tern", line 10, in <module>
    sys.exit(main())
  File "/home/rjudge/ternenv/tern/tern/__main__.py", line 201, in main
    do_main(args)
  File "/home/rjudge/ternenv/tern/tern/__main__.py", line 90, in do_main
    run.execute_dockerfile(args)
  File "/home/rjudge/ternenv/tern/tern/analyze/docker/run.py", line 176, in execute_dockerfile
    output = dockerfile.create_locked_dockerfile(dfobj)
  File "/home/rjudge/ternenv/tern/tern/analyze/docker/dockerfile.py", line 283, in create_locked_dockerfile
    expand_add_command(dfobj)
  File "/home/rjudge/ternenv/tern/tern/analyze/docker/dockerfile.py", line 270, in expand_add_command
    dockerfile_path)
  File "/home/rjudge/ternenv/tern/tern/analyze/docker/dockerfile.py", line 256, in find_git_info
    comment_line = common.check_git_src(dockerfile_path)
  File "/home/rjudge/ternenv/tern/tern/analyze/common.py", line 536, in check_git_src
    path_to_toplevel = get_git_toplevel(dockerfile_folder_path)
  File "/home/rjudge/ternenv/tern/tern/analyze/common.py", line 618, in get_git_toplevel
    command, stderr=subprocess.DEVNULL, cwd=path)
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 423, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.6/subprocess.py", line 729, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.6/subprocess.py", line 1364, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '': ''

Expected behavior
Tern should handle the error if the file doesn't exist and append

Environment you are running Tern on
Enter all that apply

• Output of 'tern --version'

Tern at commit a6fde9e32fc20097815438b23a852b009b831b47
   python version = 3.6.9 (default, Nov  7 2019, 10:44:02)

• Operating System (Linux Distro and version or Mac or Windows)

Ubuntu VM

[rnjudge]

@ForgetMe17 Can you take a look at this? It's my understanding that if the file doesn't exist, find_git_info should add a comment accordingly but it seems to parse the file name incorrectly. This file exists when the command was run, so I would expect the git information to be appended to the comment.

[rnjudge]

The following change fixes the first issue:

diff --git a/tern/analyze/common.py b/tern/analyze/common.py
index c5f1517..eed42f0 100644
--- a/tern/analyze/common.py
+++ b/tern/analyze/common.py
@@ -531,7 +531,7 @@ def check_git_src(dockerfile_path):
         - dir2/dockerfile
     So we only use dockerfile_path to find the git repo info.'''
     # get the path of the folder containing the dockerfile
-    dockerfile_folder_path = os.path.dirname(dockerfile_path)
+    dockerfile_folder_path = os.path.dirname(os.path.abspath(dockerfile_path))
     # locate the top level directory
     path_to_toplevel = get_git_toplevel(dockerfile_folder_path)
     # get the path of the target folder or file
@@ -618,6 +618,6 @@ def get_git_toplevel(path):
             command, stderr=subprocess.DEVNULL, cwd=path)
         if isinstance(output, bytes):
             path_to_toplevel = output.decode('utf-8').split('\n').pop(0)
-    except subprocess.CalledProcessError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         logger.debug("Cannot find git repo toplevel, path is %s", path)
     return path_to_toplevel

Explanation of changes:
The first issue was that if the ADD line to expand was ADD Dockerfile /tmp, we were using the relative path of the Dockerfile when we need to use the absolute. The following line in the check_git_src() function intern/analyze/common.py was returning an empty string since it was looking for the directory of the Dockerfile argument. This was then passed to get_git_toplevel(): dockerfile_folder_path = os.path.dirname(dockerfile_path) throwing the FileNotFoundError. To fix, we need to get the dirname of the absolute path of the dockerfile_path and catch the FileNotFoundError error as a possible exception. With these two changes -- the comment_line in line 256 of tern/analyze/docker/dockerfile.py correctly returns the following before trying to get the git_url:

comment_line= git project name: tern, HEAD sha: a6fde9e32fc20097815438b23a852b009b831b47

Once the above changes are merged -- another error occurs finding the git_url:

  File "/home/rjudge/ternenv/tern/tern/analyze/docker/dockerfile.py", line 284, in create_locked_dockerfile
    expand_add_command(dfobj)
  File "/home/rjudge/ternenv/tern/tern/analyze/docker/dockerfile.py", line 271, in expand_add_command
    dockerfile_path)
  File "/home/rjudge/ternenv/tern/tern/analyze/docker/dockerfile.py", line 259, in find_git_info
    url_list = common.get_git_url(dockerfile_path)
  File "/home/rjudge/ternenv/tern/tern/analyze/common.py", line 574, in get_git_url
    command, stderr=subprocess.DEVNULL, cwd=dockerfile_folder_path)
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 423, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.6/subprocess.py", line 729, in __init__
    restore_signals, start_new_session)
  File "/usr/lib/python3.6/subprocess.py", line 1364, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '': ''

The fix for the above error is the same as the first -- we must get the absolute path of the dockerfile_path and add the FileNotFoundError to the list of exceptions.

@@ -567,7 +567,7 @@ def get_git_url(dockerfile_path):
     '''Given a dockerfile_path, return url of git project which contains
     the dockerfile in a form of list.'''
     # get the path of the folder containing the dockerfile
-    dockerfile_folder_path = os.path.dirname(dockerfile_path)
+    dockerfile_folder_path = os.path.dirname(os.path.abspath(dockerfile_path))
     command = ['git', 'remote', '-v']
     try:
         output = subprocess.check_output(  # nosec
@@ -581,7 +581,7 @@ def get_git_url(dockerfile_path):
                 extract_url = extract_git_url_from_line(line)
                 if extract_url:
                     url_list.add(extract_url)
-    except subprocess.CalledProcessError:
+    except (subprocess.CalledProcessError, FileNotFoundError):
         logger.debug("Cannot find git repo url, path is %s",
                      dockerfile_folder_path)
     return url_list
@@ -618,6 +618,6 @@ def get_git_toplevel(path):
             command, stderr=subprocess.DEVNULL, cwd=path)
         if isinstance(output, bytes):
             path_to_toplevel = output.decode('utf-8').split('\n').pop(0)

With the above changes, we can get the correct comment appended to the ADD line in the Dockerfile:

ADD Dockerfile /tmp # git project name: tern, HEAD sha: a6fde9e32fc20097815438b23a852b009b831b47, project url(s): github.com/tern-tools/tern, github.com/rnjudge/tern

[rnjudge]

@ForgetMe17 Can you take a look at this? It's my understanding that if the file doesn't exist, find_git_info should add a comment accordingly but it seems to parse the file name incorrectly. This file exists when the command was run, so I would expect the git information to be appended to the comment.

Nevermind, I was able to look at this today :)

[ForgetMe17]

Thanks for finding this bug and help me fix it. i miss it somehow

[rnjudge]

Thanks for finding this bug and help me fix it. i miss it somehow

We also missed this while reviewing the PR :) It is all part of the development process.