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

Inconsistent paths - absolute vs. relative #47

Open
tihomir-kit opened this issue Sep 17, 2014 · 4 comments
Open

Inconsistent paths - absolute vs. relative #47

tihomir-kit opened this issue Sep 17, 2014 · 4 comments
Labels

Comments

@tihomir-kit
Copy link

Hi,

we're using your framework and stumbled upon the problem where File.Exists(localPath); from *Condition classes returns false because our app was not started manually (by clicking on an exe) but through registry upon machine startup.

After some debugging we found out that the problem was System.IO.File would look at a different default folder based on the two different ways of starting the app. If the app is started manually it properly looks into its root and updates the changed files. If however the app is started automatically on startup, the default directory that System.IO.File uses is for some reason C:\Windows\SysWOW64.

This caused us some problems so we temporarily fixed NAppUpdate by adding these few lines:

var appDirectory = Path.GetDirectoryName(UpdateManager.Instance.ApplicationPath);
if (!localPath.StartsWith(appDirectory, StringComparison.InvariantCultureIgnoreCase))
{
    localPath = Path.Combine(appDirectory, localPath);
}

after:

if (string.IsNullOrEmpty(localPath))
    return true;

in the following classes:

  • FileChecksumCondition
  • FileDateCondition
  • FileExistsCondition
  • FileSizeCondition
  • FileVersionCondition

A better approach would be do the append the path to a more generic place such as SetNauAttributes(INauFieldsHolder fieldsHolder, Dictionary<string, string> attributes) method from Reflection.cs but we didn't want to do this as we didn't want to break some other functionality (we saw that in some places you actually did something similar to what we did (for example in FileUpdateTask class).

The mix of absolute/relative behavior should probably be refactored and standardized so it wouldn't cause the kind of problems we experienced.

Anyway, hope this helps if someone else stumbles into same problem or until this (hopefuly ;) gets fixed/refactored in a future version of the framework.

Also, thanks for creating the framework! :)
It helped us a lot.

Cheers

@synhershko
Copy link
Owner

Thanks - haven't noticed it before but it can definitely explain some issues people were having. If you could send a PR fixing / standardising this I will be more than happy to pull it in.

I think the correct approach would be to always normalize paths to be based on the app path - absolute paths don't really make sense. We just have to be careful not to override complex folder structure.

@tihomir-kit
Copy link
Author

Yes, "based on the app paths" was what I was aiming at.

When I find some free time, I could look into the problem and try and fix it. I will probably need to ping you about a few questions, I'll need some answers about a few parts of your code to make sure I don't break anything in the process. Would that be ok?

@synhershko
Copy link
Owner

Sure, thanks in advnce

@veraciousnottaken
Copy link

Looking forward for fix.
For example my project structure requires updating:

\Bin (where app resides)
\Config
\Img
Start.bat (from here application is started)

And it was not possible without modify the sources (updater and Feedbuilder).

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

No branches or pull requests

4 participants