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

Always decode manifests with utf-8; not just most of the time #711

Merged
merged 3 commits into from
Jun 26, 2024
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
4 changes: 2 additions & 2 deletions src/west/app/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ def bootstrap(self, args) -> Path:
# Parse the manifest to get "self: path:", if it declares one.
# Otherwise, use the URL. Ignore imports -- all we really
# want to know is if there's a "self: path:" or not.
manifest = Manifest.from_data(temp_manifest.read_text(),
manifest = Manifest.from_data(temp_manifest.read_text(encoding=Manifest.encoding),
import_flags=ImportFlag.IGNORE)
if manifest.yaml_path:
manifest_path = manifest.yaml_path
Expand Down Expand Up @@ -1049,7 +1049,7 @@ def update_importer(self, project, path):
self.updated.add(project.name)

try:
return _manifest_content_at(project, path)
return _manifest_content_at(project, path, Manifest.encoding)
except FileNotFoundError:
# FIXME we need each project to have back-pointers
# to the manifest file where it was defined, so we can
Expand Down
24 changes: 15 additions & 9 deletions src/west/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ def _west_commands_merge(wc1: List[str], wc2: List[str]) -> List[str]:
def _default_importer(project: 'Project', file: str) -> NoReturn:
raise ManifestImportFailed(project, file)

def _manifest_content_at(project: 'Project', path: PathType,
def _manifest_content_at(project: 'Project', path: PathType, mf_encoding: str,
rev: str = QUAL_MANIFEST_REV_BRANCH) \
-> ImportedContentType:
# Get a list of manifest data from project at path
Expand Down Expand Up @@ -272,19 +272,22 @@ def _manifest_content_at(project: 'Project', path: PathType,
# enough!
raise OSError(errno.ENOENT, os.strerror(errno.ENOENT), path)

ptype = out.decode('utf-8').split()[1]
# TODO: does git always output utf-8? Test git config 'core.precomposeunicode' etc.
git_filenames_encoding = 'utf-8'
ptype = out.decode(git_filenames_encoding).split()[1]

if ptype == 'blob':
# Importing a file: just return its content.
return project.read_at(path, rev=rev).decode('utf-8')
return project.read_at(path, rev=rev).decode(mf_encoding)
elif ptype == 'tree':
# Importing a tree: return the content of the YAML files inside it.
ret = []
# Use a PurePosixPath because that's the form git seems to
# store internally, even on Windows.
pathobj = PurePosixPath(path)
for f in filter(_is_yml, project.listdir_at(path, rev=rev)):
ret.append(project.read_at(pathobj / f, rev=rev).decode('utf-8'))
for f in filter(_is_yml, project.listdir_at(path, rev=rev,
encoding=git_filenames_encoding)):
ret.append(project.read_at(pathobj / f, rev=rev).decode(mf_encoding))
return ret
else:
raise MalformedManifest(f"can't decipher project {project.name} "
Expand Down Expand Up @@ -1184,6 +1187,9 @@ class Manifest:
'''The parsed contents of a west manifest file.
'''

# TODO: make this a new west config: 'manifest.encoding'
encoding: str = 'utf-8'

@staticmethod
def from_topdir(topdir: Optional[PathType] = None,
config: Optional[Configuration] = None,
Expand Down Expand Up @@ -1902,7 +1908,7 @@ def get_option(option, default=None):
current_relpath = manifest_path / manifest_file
current_abspath = topdir_abspath / current_relpath
try:
current_data = current_abspath.read_text(encoding='utf-8')
current_data = current_abspath.read_text(encoding=Manifest.encoding)
except FileNotFoundError:
raise MalformedConfig(
f'file not found: manifest file {current_abspath} '
Expand Down Expand Up @@ -2201,7 +2207,7 @@ def _import_pathobj_from_self(self, pathobj_abs: Path,
child_ctx = self._ctx._replace(
current_abspath=pathobj_abs,
current_relpath=pathobj,
current_data=pathobj_abs.read_text(encoding='utf-8')
current_data=pathobj_abs.read_text(encoding=Manifest.encoding)
)
try:
Manifest(topdir=self.topdir, internal_import_ctx=child_ctx)
Expand Down Expand Up @@ -2239,7 +2245,7 @@ def _import_map_from_self(self, imp: Dict) -> None:
path_prefix=path_prefix,
current_abspath=import_abs,
current_relpath=pathobj / import_abs.name,
current_data=import_abs.read_text(encoding='utf-8')
current_data=import_abs.read_text(encoding=Manifest.encoding)
)
try:
Manifest(topdir=self.topdir, internal_import_ctx=child_ctx)
Expand Down Expand Up @@ -2572,7 +2578,7 @@ def _import_content_from_project(self, project: Project,
if not (self._ctx.import_flags & ImportFlag.FORCE_PROJECTS) and \
project.is_cloned():
try:
content = _manifest_content_at(project, path)
content = _manifest_content_at(project, path, Manifest.encoding)
except MalformedManifest as mm:
self._malformed(mm.args[0])
except FileNotFoundError:
Expand Down
Loading