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

nix flake: clarify error message when file is an unknown type #12167

Open
wants to merge 1 commit 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
6 changes: 5 additions & 1 deletion src/libstore/nar-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,11 @@ json listNar(ref<SourceAccessor> accessor, const CanonPath & path, bool recurse)
obj["type"] = "symlink";
obj["target"] = accessor->readLink(path);
break;
case SourceAccessor::Type::tMisc:
case SourceAccessor::Type::tBlock:
case SourceAccessor::Type::tChar:
case SourceAccessor::Type::tSocket:
case SourceAccessor::Type::tFifo:
case SourceAccessor::Type::tUnknown:
assert(false); // cannot happen for NARs
}
return obj;
Expand Down
10 changes: 6 additions & 4 deletions src/libutil/fs-sink.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,13 @@ void copyRecursive(
break;
}

case SourceAccessor::tMisc:
throw Error("file '%1%' has an unsupported type", from);

case SourceAccessor::tChar:
case SourceAccessor::tBlock:
case SourceAccessor::tSocket:
case SourceAccessor::tFifo:
case SourceAccessor::tUnknown:
default:
unreachable();
throw Error("file '%1%' has an unsupported type of %2%", from, stat.typeString());
}
}

Expand Down
12 changes: 10 additions & 2 deletions src/libutil/git.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ std::optional<Mode> convertMode(SourceAccessor::Type type)
case SourceAccessor::tSymlink: return Mode::Symlink;
case SourceAccessor::tRegular: return Mode::Regular;
case SourceAccessor::tDirectory: return Mode::Directory;
case SourceAccessor::tMisc: return std::nullopt;
case SourceAccessor::tChar:
case SourceAccessor::tBlock:
case SourceAccessor::tSocket:
case SourceAccessor::tFifo: return std::nullopt;
case SourceAccessor::tUnknown:
default: unreachable();
}
}
Expand Down Expand Up @@ -314,7 +318,11 @@ Mode dump(
return Mode::Symlink;
}

case SourceAccessor::tMisc:
case SourceAccessor::tChar:
case SourceAccessor::tBlock:
case SourceAccessor::tSocket:
case SourceAccessor::tFifo:
case SourceAccessor::tUnknown:
Comment on lines +321 to +325
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
case SourceAccessor::tChar:
case SourceAccessor::tBlock:
case SourceAccessor::tSocket:
case SourceAccessor::tFifo:
case SourceAccessor::tUnknown:

We can just rely on the default case here (everything is unsupported except for the types handled above).

default:
throw Error("file '%1%' has an unsupported type", path);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
throw Error("file '%1%' has an unsupported type", path);
throw Error("file '%1%' has an unsupported type", path);

This can use typeString() now to show the type of the file.

}
Expand Down
12 changes: 10 additions & 2 deletions src/libutil/posix-source-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,11 @@ std::optional<SourceAccessor::Stat> PosixSourceAccessor::maybeLstat(const CanonP
S_ISREG(st->st_mode) ? tRegular :
S_ISDIR(st->st_mode) ? tDirectory :
S_ISLNK(st->st_mode) ? tSymlink :
tMisc,
S_ISCHR(st->st_mode) ? tChar :
S_ISBLK(st->st_mode) ? tBlock :
S_ISSOCK(st->st_mode) ? tSocket :
S_ISFIFO(st->st_mode) ? tFifo :
tUnknown,
.fileSize = S_ISREG(st->st_mode) ? std::optional<uint64_t>(st->st_size) : std::nullopt,
.isExecutable = S_ISREG(st->st_mode) && st->st_mode & S_IXUSR,
};
Expand Down Expand Up @@ -156,7 +160,11 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
case std::filesystem::file_type::regular: return Type::tRegular; break;
case std::filesystem::file_type::symlink: return Type::tSymlink; break;
case std::filesystem::file_type::directory: return Type::tDirectory; break;
default: return tMisc;
case std::filesystem::file_type::character: return Type::tChar; break;
case std::filesystem::file_type::block: return Type::tBlock; break;
case std::filesystem::file_type::fifo: return Type::tFifo; break;
case std::filesystem::file_type::socket: return Type::tSocket; break;
default: return tUnknown;
}
#pragma GCC diagnostic pop
}();
Expand Down
19 changes: 19 additions & 0 deletions src/libutil/source-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,25 @@ namespace nix {

static std::atomic<size_t> nextNumber{0};

bool SourceAccessor::Stat::isTypeUnknown() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
bool SourceAccessor::Stat::isTypeUnknown() {
bool SourceAccessor::Stat::isTypeUnknown()
{

Formatting nitpick.

isTypeUnknown() is a misnomer since we do know the type now. Maybe it should be something like isNotNARSerialisable() to denote that it's a type than isn't supported in a NAR file. (Or isNARSerialisable() to avoid the negation.)

return this->type != tRegular && this->type != tSymlink && this->type != tDirectory;
}

std::string SourceAccessor::Stat::typeString() {
switch (this->type) {
case tRegular: return "regular";
case tSymlink: return "symlink";
case tDirectory: return "directory";
case tChar: return "character device";
case tBlock: return "block device";
case tSocket: return "socket";
case tFifo: return "fifo";
case tUnknown:
default: return "unknown";
}
return "unknown";
}

SourceAccessor::SourceAccessor()
: number(++nextNumber)
, displayPrefix{"«unknown»"}
Expand Down
8 changes: 6 additions & 2 deletions src/libutil/source-accessor.hh
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,13 @@ struct SourceAccessor : std::enable_shared_from_this<SourceAccessor>
Unlike `DT_UNKNOWN`, this must not be used for deferring the lookup of types.
*/
tMisc
tChar, tBlock, tSocket, tFifo,
tUnknown
};

struct Stat
{
Type type = tMisc;
Type type = tUnknown;

/**
* For regular files only: the size of the file. Not all
Expand All @@ -112,6 +113,9 @@ struct SourceAccessor : std::enable_shared_from_this<SourceAccessor>
* file in the NAR. Only returned by NAR accessors.
*/
std::optional<uint64_t> narOffset;

bool isTypeUnknown();
std::string typeString();
};

Stat lstat(const CanonPath & path);
Expand Down
2 changes: 1 addition & 1 deletion src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -941,7 +941,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand
createSymlink(target, os_string_to_string(PathViewNG { to2 }));
}
else
throw Error("file '%s' has unsupported type", from2);
throw Error("path '%s' needs to be a symlink, file, or directory but instead is a %s", from2, st.typeString());
changedFiles.push_back(to2);
notice("wrote: %s", to2);
}
Expand Down
Loading