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

base_parser: support relative path include files #399

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
28 changes: 26 additions & 2 deletions edk2toollib/uefi/edk2/parsers/base_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@
self.PPs = []
self._Edk2PathUtil = None
self.TargetFilePath = None # the abs path of the target file
self.FilePathStack = [] # a stack containing a list of size 2: [filepath, line_count]
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT:

Since you're likely dealing with a large set of data, the following solution probably doesn't make a lot of sense performance wise but if you wanted to improve readability of the code you could do something like a namedtuple.

However the object instantiation might be too much for a large dataset

from collections import namedtuple

# Define the namedtuple type
FilePathRecord = namedtuple('FilePathRecord', ['filepath', 'line_count'])

self.FilePathStack = []  # a stack containing FilePathRecord instances

self.FilePathStack.append(FilePathRecord('my_file_path', 100))

self.FilePathStack[0].file_path

self.ParsedFiles = set()
self.CurrentLine = -1
self._MacroNotDefinedValue = "0" # value to used for undefined macro

Expand Down Expand Up @@ -133,6 +135,27 @@
self.InputVars = inputdict
return self

def PushTargetFile(self, abs_path, line_count):
"""Adds a target file to the stack."""
self.FilePathStack.append([abs_path, line_count])
self.ParsedFiles.add(abs_path)

def DecrementLinesParsed(self) -> bool:
"""Decrements line count for the current target file by one.

Returns:
(bool): True if there are still lines to parse, False otherwise.
"""
if not self.FilePathStack:
return False

Check warning on line 150 in edk2toollib/uefi/edk2/parsers/base_parser.py

View check run for this annotation

Codecov / codecov/patch

edk2toollib/uefi/edk2/parsers/base_parser.py#L150

Added line #L150 was not covered by tests
line_count = self.FilePathStack[-1][1]
if line_count - 1 > 0:
self.FilePathStack[-1][1] -= 1
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT:

        updated_line_count = self.FilePathStack[-1][1] - 1
        if updated_line_count > 0:
            self.FilePathStack[-1][1] -= updated_line_count

return True

self.FilePathStack.pop()
return False

def FindPath(self, *p):
"""Given a path, it will find it relative to the root, the current target file, or the packages path.

Expand All @@ -159,8 +182,8 @@
return os.path.abspath(Path)

# If that fails, check a path relative to the target file.
if self.TargetFilePath is not None:
Path = os.path.abspath(os.path.join(os.path.dirname(self.TargetFilePath), *p))
if self.FilePathStack:
Path = os.path.abspath(os.path.join(os.path.dirname(self.FilePathStack[-1][0]), *p))
if os.path.exists(Path):
return os.path.abspath(Path)

Expand Down Expand Up @@ -789,6 +812,7 @@
self.ConditionalStack = []
self.CurrentSection = ''
self.CurrentFullSection = ''
self.FilePathStack = []
self.Parsed = False


Expand Down
16 changes: 7 additions & 9 deletions edk2toollib/uefi/edk2/parsers/dsc_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ def __ParseLine(self, Line, file_name=None, lineno=None):
if sp is None:
raise FileNotFoundError(include_file)
self.Logger.debug("Opening Include File %s" % sp)
self._PushTargetFile(sp)
lf = open(sp, "r")
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: I know you didn't write this but if we switch to with we can drop lf.close()

loc = lf.readlines()
lf.close()
self.PushTargetFile(sp, len(loc))
return ("", loc, sp)

# check for new section
Expand Down Expand Up @@ -216,10 +216,10 @@ def __ParseDefineLine(self, Line):
sp = self.FindPath(include_file)
if sp is None:
raise FileNotFoundError(include_file)
self._PushTargetFile(sp)
lf = open(sp, "r")
loc = lf.readlines()
lf.close()
self.PushTargetFile(sp, len(loc))
return ("", loc)

# check for new section
Expand Down Expand Up @@ -293,6 +293,7 @@ def __ProcessMore(self, lines, file_name=None):
# otherwise, let the user know that we failed in the DSC
self.Logger.warning(f"DSC Parser (No-Fail Mode): {raw_line}")
self.Logger.warning(e)
self.DecrementLinesParsed()

def __ProcessDefines(self, lines):
"""Goes through a file once to look for [Define] sections.
Expand All @@ -315,6 +316,7 @@ def __ProcessDefines(self, lines):
# otherwise, raise the exception and act normally
if not self._no_fail_mode:
raise
self.DecrementLinesParsed()
# Reset the PcdValueDict as this was just to find any Defines.
self.PcdValueDict = {}

Expand Down Expand Up @@ -477,25 +479,21 @@ def ParseFile(self, filepath):
sp = self.FindPath(filepath)
if sp is None:
raise FileNotFoundError(filepath)
self._PushTargetFile(sp)
f = open(sp, "r")
# expand all the lines and include other files
file_lines = f.readlines()
self.PushTargetFile(sp, len(file_lines))
self.__ProcessDefines(file_lines)
# reset the parser state before processing more
self.ResetParserState()
self._PushTargetFile(sp)
self.PushTargetFile(sp, len(file_lines))
self.__ProcessMore(file_lines, file_name=sp)
f.close()

self._parse_libraries()
self._parse_components()
self.Parsed = True

def _PushTargetFile(self, targetFile):
self.TargetFilePath = os.path.abspath(targetFile)
self._dsc_file_paths.add(self.TargetFilePath)

def GetMods(self):
"""Returns a list with all Mods."""
return self.ThreeMods + self.SixMods
Expand All @@ -517,7 +515,7 @@ def GetAllDscPaths(self):

They are not all guaranteed to be DSC files
"""
return self._dsc_file_paths
return self.ParsedFiles

def RegisterPcds(self, line):
"""Reads the line and registers any PCDs found."""
Expand Down
11 changes: 9 additions & 2 deletions edk2toollib/uefi/edk2/parsers/fdf_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,16 @@ def GetNextLine(self):
sline = self.StripComment(line)

if (sline is None or len(sline) < 1):
self.DecrementLinesParsed()
return self.GetNextLine()

sline = self.ReplaceVariables(sline)
if self.ProcessConditional(sline):
# was a conditional so skip
self.DecrementLinesParsed()
return self.GetNextLine()
if not self.InActiveCode():
self.DecrementLinesParsed()
return self.GetNextLine()

self._BracketCount += sline.count("{")
Expand All @@ -67,9 +70,11 @@ def GetNextLine(self):
def InsertLinesFromFile(self, file_path: str):
"""Adds additional lines to the Lines Attribute from the provided file."""
with open(file_path, 'r') as lines_file:
self.Lines += reversed(lines_file.readlines())
lines = lines_file.readlines()
self.Lines += reversed(lines)
# Back off the line count to ignore the include line itself.
self.CurrentLine -= 1
self.PushTargetFile(file_path, len(lines))

def ParseFile(self, filepath):
"""Parses the provided FDF file."""
Expand All @@ -79,12 +84,13 @@ def ParseFile(self, filepath):
else:
fp = filepath
self.Path = fp
self.TargetFilePath = os.path.abspath(fp)
fp = os.path.abspath(fp)
self.CurrentLine = 0
self._f = open(fp, "r")
self.Lines = self._f.readlines()
self.Lines.reverse()
self._f.close()
self.PushTargetFile(fp, len(self.Lines))
self._BracketCount = 0
InDefinesSection = False
InFdSection = False
Expand Down Expand Up @@ -221,4 +227,5 @@ def ParseFile(self, filepath):
elif sline.strip().lower().startswith('[rule.'):
InRuleSection = True

self.DecrementLinesParsed()
self.Parsed = True
13 changes: 7 additions & 6 deletions tests.unit/parsers/test_base_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@
# SPDX-License-Identifier: BSD-2-Clause-Patent
##

import os
import tempfile
import unittest

from edk2toollib.uefi.edk2.parsers.base_parser import BaseParser
from edk2toollib.uefi.edk2.path_utilities import Edk2Path
import tempfile
import os


class TestBaseParser(unittest.TestCase):
Expand Down Expand Up @@ -460,22 +461,22 @@ def test_emulator_conditional_not_in(self):
parser.ResetParserState()

def test_emulator_conditional_parens_order(self):
''' Makes sure the parenthesis affect the order of expressions '''
'''Makes sure the parenthesis affect the order of expressions.'''
parser = BaseParser("")
self.assertFalse(parser.EvaluateConditional('!if TRUE OR FALSE AND FALSE'))
self.assertTrue(parser.EvaluateConditional('!if TRUE OR (FALSE AND FALSE)'))
parser.ResetParserState()

def test_emulator_conditional_not_or(self):
''' Makes sure we can use the not with other operators '''
'''Makes sure we can use the not with other operators.'''
parser = BaseParser("")
self.assertTrue(parser.EvaluateConditional('!if FALSE NOT OR FALSE'))
self.assertFalse(parser.EvaluateConditional('!if TRUE NOT OR FALSE'))
self.assertFalse(parser.EvaluateConditional('!if FALSE NOT OR TRUE'))
self.assertFalse(parser.EvaluateConditional('!if TRUE NOT OR TRUE'))

def test_emulator_conditional_not_it_all(self):
''' Makes sure the parenthesis affect the order of expressions '''
'''Makes sure the parenthesis affect the order of expressions.'''
parser = BaseParser("")
self.assertTrue(parser.EvaluateConditional('!if NOT FALSE OR FALSE'))
self.assertFalse(parser.EvaluateConditional('!if NOT TRUE OR FALSE'))
Expand Down Expand Up @@ -640,7 +641,7 @@ def test_find_path(self):
# create target file
os.makedirs(target_filedir)
parser.WriteLinesToFile(target_filepath)
parser.TargetFilePath = target_filepath
parser.PushTargetFile(target_filepath, 0)
# check if we can find the root
root_found = parser.FindPath(root_file)
self.assertEqual(root_found, root_filepath)
Expand Down
57 changes: 50 additions & 7 deletions tests.unit/parsers/test_dsc_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
# SPDX-License-Identifier: BSD-2-Clause-Patent
##

import unittest
import tempfile
import os
import tempfile
import unittest

from edk2toollib.uefi.edk2.parsers.dsc_parser import DscParser
from edk2toollib.uefi.edk2.path_utilities import Edk2Path

Expand All @@ -30,7 +31,7 @@ def write_to_file(file_path, data):
f.close()

def test_dsc_include_single_file(self):
''' This tests whether includes work properly '''
'''This tests whether includes work properly.'''
workspace = tempfile.mkdtemp()

file1_name = "file1.dsc"
Expand All @@ -53,7 +54,7 @@ def test_dsc_include_single_file(self):
self.assertEqual(len(parser.GetAllDscPaths()), 2) # make sure we have two dsc paths

def test_dsc_include_missing_file(self):
''' This tests whether includes work properly '''
'''This tests whether includes work properly.'''
workspace = tempfile.mkdtemp()

file1_name = "file1.dsc"
Expand All @@ -69,7 +70,7 @@ def test_dsc_include_missing_file(self):
parser.ParseFile(file1_path)

def test_dsc_include_missing_file_no_fail_mode(self):
''' This tests whether includes work properly if no fail mode is on'''
'''This tests whether includes work properly if no fail mode is on.'''
workspace = tempfile.mkdtemp()

file1_name = "file1.dsc"
Expand All @@ -85,7 +86,7 @@ def test_dsc_include_missing_file_no_fail_mode(self):
parser.ParseFile(file1_path)

def test_dsc_parse_file_on_package_path(self):
''' This tests whether includes work properly if no fail mode is on'''
'''This tests whether includes work properly if no fail mode is on.'''
workspace = tempfile.mkdtemp()
working_dir_name = "working"
working2_dir_name = "working2"
Expand Down Expand Up @@ -113,7 +114,7 @@ def test_dsc_parse_file_on_package_path(self):
self.assertEqual(parser.LocalVars["INCLUDED"], "TRUE") # make sure we got the defines

def test_dsc_include_relative_path(self):
''' This tests whether includes work properly with a relative path'''
'''This tests whether includes work properly with a relative path.'''
workspace = tempfile.mkdtemp()
outside_folder = os.path.join(workspace, "outside")
inside_folder = os.path.join(outside_folder, "inside")
Expand Down Expand Up @@ -154,3 +155,45 @@ def test_dsc_include_relative_path(self):
self.assertEqual(parser.LocalVars["INCLUDED"], "TRUE") # make sure we got the defines
finally:
os.chdir(cwd)

def test_dsc_include_relative_paths2(tmp_path):
"""This tests whether includes work properly with a relative path, when includes are not nested.

Directory setup
src
└─ Platforms
└─ PlatformPkg
├─ PlatformPkg.dsc
└─ includes
├─ PCDs1.dsc.inc
└─ PCDs2.dsc.inc

Base DSC
[PcdsFixedAtBuild]
gPlatformPkgTokenSpaceGuid.PcdSomething
!include includes/PCDs1.dsc.inc
!include includes/PCDs2.dsc.inc
"""
pp = tmp_path / "Platforms"
pkg_dir = pp / "PlatformPkg"
dsc = pkg_dir / "PlatformPkg.dsc"
inc_dir = pkg_dir / "includes"

# Create Platforms, Platforms/PlatformPkg, Platforms/PlatformPkg/includes
inc_dir.mkdir(parents=True)

with open(inc_dir / "PCDs1.dsc.inc", "w") as f:
f.write("gPlatformPkgTokenSpaceGuid.PcdSomething1\n")

with open(inc_dir / "PCDs2.dsc.inc", "w") as f:
f.write("gPlatformPkgTokenSpaceGuid.PcdSomething2\n")

with open(dsc, "w") as f:
f.write("[PcdsFixedAtBuild]\n")
f.write("gPlatformPkgTokenSpaceGuid.PcdSomething\n")
f.write("!include includes/PCDs1.dsc.inc\n")
f.write("!include includes/PCDs2.dsc.inc\n")

parser = DscParser()
parser.SetEdk2Path(Edk2Path(str(tmp_path), [str(pp)]))
parser.ParseFile(dsc)
Loading