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

fix for include-referenced-projects flag #2753

Merged
merged 1 commit into from
Sep 14, 2017
Merged

Conversation

lexarchik
Copy link
Contributor

Try to fix #1848

[if includeReferencedProjects then
for proj in project.GetAllReferencedProjects(false,projDeps) |> Seq.filter ((<>) project) do
match proj.FindTemplatesFile() with
| Some templateFileName when TemplateFile.IsProjectType templateFileName ->
match TemplateFile.Load(templateFileName, lockFile, None, Seq.empty |> Map.ofSeq).Contents with
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea of this fix is here: we can't just Load template file for referenced project - we must merge it with info from project (we do so in merge method in PackageProcess.fs).

| None -> true

Path.GetFullPath projectFile.FileName |> normalizePath,(merged,projectFile,willBePacked))
|> Map.ofArray
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we pack single template findDependencies still need all project-template info for choose right version of nuget dependencies if flag --include-referenced-projects was provided.

Copy link
Member

@matthid matthid left a comment

Choose a reason for hiding this comment

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

I'm not really that familiar with that part of the code base as I rarely use paket pack...

Generally if the tests make sense we probably should take it. It looks really well made from the amount of tests, I have noted only one minor personal preference regarding tuples...

@@ -159,7 +159,7 @@ let addFile (source : string) (target : string) (templateFile : TemplateFile) =
| IncompleteTemplate ->
failwith (sprintf "You should only try and add files to template files with complete metadata.%sFile: %s" Environment.NewLine templateFile.FileName)

let findDependencies (dependenciesFile : DependenciesFile) config platform (template : TemplateFile) (project : ProjectFile) lockDependencies minimumFromLockFile pinProjectReferences (map : Map<string, TemplateFile * ProjectFile>) includeReferencedProjects (version :SemVerInfo option) specificVersions (projDeps) =
let findDependencies (dependenciesFile : DependenciesFile) config platform (template : TemplateFile) (project : ProjectFile) lockDependencies minimumFromLockFile pinProjectReferences (projectWithTemplates : Map<string, TemplateFile * ProjectFile * bool>) includeReferencedProjects (version :SemVerInfo option) specificVersions (projDeps) =
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to introduce records instead of triples..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i will try to change it next time:)

@forki forki merged commit 9e3f9c7 into fsprojects:master Sep 14, 2017
@lexarchik lexarchik deleted the fix_1848 branch September 14, 2017 11:14
@lexarchik lexarchik mentioned this pull request Sep 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

include-referenced-projects lead to strange changes in nuspec
3 participants