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

Cake Frosting Parent DirectoryPath Fails To Combine with Slash #3352

Closed
2 tasks done
lprichar opened this issue May 12, 2021 · 7 comments · Fixed by #3353
Closed
2 tasks done

Cake Frosting Parent DirectoryPath Fails To Combine with Slash #3352

lprichar opened this issue May 12, 2021 · 7 comments · Fixed by #3353
Labels
Milestone

Comments

@lprichar
Copy link

Prerequisites

  • I have written a descriptive issue title
  • I have searched issues to ensure it has not already been reported

Cake runner

Cake Frosting

Cake version

1.1.0

Operating system

Linux, Windows

Operating system architecture

64-Bit

CI Server

No response

What are you seeing?

A NullReferenceException in the code sample below. However, that's more the symptom than the cause. The problem appears to be that Cake Frosting is combining Directory("..") + Directory("temp") into "..temp" rather than "../temp" when you capture the context and then use expression bodied properties.

This may seem obscure, but I'm converting a large codebase of DSL Cake into Frosting and am looking for ways to accomplish it with minimal changes.

What is expected?

It should run to completion without an exception

Steps to Reproduce

Run this:

[TaskName("Fail")]
public class FailTask : FrostingTask<BuildContext>
{
    private BuildContext? _context;
    private DirectoryPath RootDir => _context.Directory("..");
    private DirectoryPath TestDir => RootDir + _context.Directory("temp");

    public override void Run(BuildContext context)
    {
        _context = context;
        context.GetFiles(TestDir + context.File("*.zip"));
    }
}

Output log

Error: System.NullReferenceException: Object reference not set to an instance of an object.
   at Cake.Core.IO.Globbing.GlobVisitor.VisitParent(ParentDirectoryNode node, GlobVisitorContext context)
   at Cake.Core.IO.Globbing.Nodes.ParentDirectoryNode.Accept(GlobVisitor visitor, GlobVisitorContext context)
   at Cake.Core.IO.Globbing.GlobVisitor.VisitRelativeRoot(RelativeRootNode node, GlobVisitorContext context)
   at Cake.Core.IO.Globbing.Nodes.RelativeRootNode.Accept(GlobVisitor globber, GlobVisitorContext context)
   at Cake.Core.IO.Globbing.GlobVisitor.Walk(GlobNode node, GlobberSettings settings)
   at Cake.Core.IO.Globber.Match(GlobPattern pattern, GlobberSettings settings)
   at Cake.Core.IO.GlobberExtensions.Match(IGlobber globber, GlobPattern pattern)
   at Cake.Common.IO.GlobbingAliases.GetFiles(ICakeContext context, GlobPattern pattern)
   ...
@patriksvensson
Copy link
Member

GetFiles takes a string so it will implicitly convert the content to a string. Try using TestDir.CombineWithFilePath(..) and you should get the correct result.

@lprichar
Copy link
Author

Didn't help, I'm afraid. More specifically:

[TaskName("Fail")]
public class FailTask : FrostingTask<BuildContext>
{
    private BuildContext? _context;
    private DirectoryPath RootDir => _context.Directory("..");
    private DirectoryPath TestDir => RootDir + _context.Directory("temp");

    public override void Run(BuildContext context)
    {
        _context = context;
        var filePath = TestDir.CombineWithFilePath(context.File("*.zip"));
        _context.Information(filePath);
    }
}

returns ..temp/*.zip. That's really the source of the problem.

I'd be inclined to argue that a null reference exception from GetFiles is never the correct response, but again that's just more a symptom of the underlying problem which is the way directories are being combined is incorrect in this context.

@lprichar
Copy link
Author

lprichar commented May 12, 2021

Found a workaround. If you replace:

private DirectoryPath TestDir => RootDir + _context.Directory("temp");

with

private DirectoryPath TestDir => RootDir.Combine(_context.Directory("temp"));

then it works. Strange bug, I can't even begin to understand how that syntax change could make a difference. Just to make this easily reproduceable:

[TaskName("Fail")]
public class FailTask : FrostingTask<BuildContext>
{
    private BuildContext? _context;
    private DirectoryPath RootDir => _context.Directory("..");
    private DirectoryPath TestDir1 => RootDir + _context.Directory("temp");
    private DirectoryPath TestDir2 => RootDir.Combine(_context.Directory("temp"));

    public override void Run(BuildContext context)
    {
        _context = context;
        _context.Information(TestDir1);
        _context.Information(TestDir2);
    }
}

returns:

..temp
../temp

but should return:

../temp
../temp

@augustoproiete
Copy link
Member

@lprichar We're missing a + operator that takes a DirectoryPath and ConvertableDirectoryPath, and ConvertableDirectoryPath has an implicit operator that casts it string so C# is effectively concatenating two strings in your case, rather than combining the file paths.

Fixed via PR #3353

@lprichar
Copy link
Author

Oh, of course! That makes sense. Thanks @augustoproiete, looking forward to the next release.

@augustoproiete
Copy link
Member

@lprichar Thanks for reporting! Nice find!

@augustoproiete augustoproiete added this to the v1.x Next Candidate milestone May 12, 2021
@cake-build cake-build deleted a comment from cake-build-bot Sep 15, 2021
augustoproiete added a commit to augustoproiete-forks/cake-build--cake that referenced this issue Oct 5, 2021
@cake-build-bot
Copy link

🎉 This issue has been resolved in version v1.3.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants