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

Paket pack skips files #2445

Closed
nikoudel opened this Issue Jun 19, 2017 · 17 comments

Comments

Projects
None yet
3 participants
@nikoudel

nikoudel commented Jun 19, 2017

Description

Starting from 5.0.0-rc009 paket skips duplicate files from the package being created.

Repro steps

  1. Create a Test.template file like this:
type file
id Paket.Test
version 1.0.0
description Paket Test
authors NK
files
    File1.txt ==> tools\Dir1
    File1.txt ==> tools\Dir2
  1. Run .\paket.exe pack output . templatefile .\Test.template

Expected behavior

The resulting package should contain both directories with File1.txt in each of them.
A real-life scenario is when a package contains multiple applications sharing some common libraries.

Actual behavior

The resulting package only contains Dir1.

image

Known workarounds

None.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

I tried to fix it in 5.1.2 but @konste you need to test as well since the relatred bug I fixed for you introduced this issue. So my fix was wrong (tbf it was a shot in the dark) and I'm not sure if now everything is well.

Member

forki commented Jun 19, 2017

I tried to fix it in 5.1.2 but @konste you need to test as well since the relatred bug I fixed for you introduced this issue. So my fix was wrong (tbf it was a shot in the dark) and I'm not sure if now everything is well.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

5.1.3 already underway with yet another fix

Member

forki commented Jun 19, 2017

5.1.3 already underway with yet another fix

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

I will test when 5.1.3 is out.

Contributor

konste commented Jun 19, 2017

I will test when 5.1.3 is out.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member
Member

forki commented Jun 19, 2017

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

Gosh... you are too fast for me! Ok, will test now.

Contributor

konste commented Jun 19, 2017

Gosh... you are too fast for me! Ok, will test now.

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

PaketPackRepro3.zip
Attached repro shows that simple bin\Release ==> Tools\ causes EXE and PDB duplication (in the same folder!). At the same time adding !bin\Release\*.exe causes both copies of EXE to disappear.
Before jumping on fixing it we need to spend a little time designing what the desired behavior should be and how to make it predictable. Most confusion is coming from the fact that list of files to copy is coming from two sources - dependencies Paket found and explicit list in files section. But negative clauses in the files section affect both sources. Let me think for a few minutes...

Contributor

konste commented Jun 19, 2017

PaketPackRepro3.zip
Attached repro shows that simple bin\Release ==> Tools\ causes EXE and PDB duplication (in the same folder!). At the same time adding !bin\Release\*.exe causes both copies of EXE to disappear.
Before jumping on fixing it we need to spend a little time designing what the desired behavior should be and how to make it predictable. Most confusion is coming from the fact that list of files to copy is coming from two sources - dependencies Paket found and explicit list in files section. But negative clauses in the files section affect both sources. Let me think for a few minutes...

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

That's what I feared. The problem is that for some reason we can't detect that the target is the same. We can definitely detect that the source is the same. But that causes trouble

Member

forki commented Jun 19, 2017

That's what I feared. The problem is that for some reason we can't detect that the target is the same. We can definitely detect that the source is the same. But that causes trouble

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

It almost feels like two sources of files should be independent. First source - dependencies and primary output of the current project is one thing - it should go to the output regardless. Files section should work within itself - it adds some files, then may be exclude some files, but exclusion should only affect what it added!
This way primary output will always go where it belongs and if somebody (like me) wants to take everything else he can add bin\Release ==> Tools\ and !bin\Release\*.exe to add everything but EXE.

Contributor

konste commented Jun 19, 2017

It almost feels like two sources of files should be independent. First source - dependencies and primary output of the current project is one thing - it should go to the output regardless. Files section should work within itself - it adds some files, then may be exclude some files, but exclusion should only affect what it added!
This way primary output will always go where it belongs and if somebody (like me) wants to take everything else he can add bin\Release ==> Tools\ and !bin\Release\*.exe to add everything but EXE.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

@konste in any case we are not allowed to add two files into same folder inside zip. but that is what is happening. trying to fix it

Member

forki commented Jun 19, 2017

@konste in any case we are not allowed to add two files into same folder inside zip. but that is what is happening. trying to fix it

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

It may sound drastic, but how about we change the logic of taking primary output only to taking everything in the output folder?

Contributor

konste commented Jun 19, 2017

It may sound drastic, but how about we change the logic of taking primary output only to taking everything in the output folder?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

doesn't fix the root cause here. maybe it would fix your issue ;-)

Member

forki commented Jun 19, 2017

doesn't fix the root cause here. maybe it would fix your issue ;-)

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

For class libraries it would not change anything - they only have DLL and PDB in the output folder. And for other types of projects it really makes sense to take everything as they probably cannot run without the rest of the files anyway.

Contributor

konste commented Jun 19, 2017

For class libraries it would not change anything - they only have DLL and PDB in the output folder. And for other types of projects it really makes sense to take everything as they probably cannot run without the rest of the files anyway.

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

The problem with the "files" section is that there is no way to tell which part of the project output is primary output and dependencies and which is everything else. It is impossible to author "files" section properly without knowledge what is already included by default. So the algorithm should look like this:

  1. Calculate the copy list from primary output and dependencies;
  2. Calculate second copy list from the "files" section;
  3. Merge the lists removing duplicates comparing target path!

This way whatever curious user writes to the "files" section he or she would only get single instance of all files in each ZIP folder.

Contributor

konste commented Jun 19, 2017

The problem with the "files" section is that there is no way to tell which part of the project output is primary output and dependencies and which is everything else. It is impossible to author "files" section properly without knowledge what is already included by default. So the algorithm should look like this:

  1. Calculate the copy list from primary output and dependencies;
  2. Calculate second copy list from the "files" section;
  3. Merge the lists removing duplicates comparing target path!

This way whatever curious user writes to the "files" section he or she would only get single instance of all files in each ZIP folder.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

image

see the issue is casing as I expected. Tools vs. tools.

so it can't dedupe it.

Member

forki commented Jun 19, 2017

image

see the issue is casing as I expected. Tools vs. tools.

so it can't dedupe it.

@konste

This comment has been minimized.

Show comment
Hide comment
@konste

konste Jun 19, 2017

Contributor

Make comparison case insensitive - what's the problem?

Contributor

konste commented Jun 19, 2017

Make comparison case insensitive - what's the problem?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

other OSes ;-) but yes I guess we need to do that for win.

Member

forki commented Jun 19, 2017

other OSes ;-) but yes I guess we need to do that for win.

forki added a commit that referenced this issue Jun 19, 2017

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 19, 2017

Member

ok. so I think both are resolved now

Member

forki commented Jun 19, 2017

ok. so I think both are resolved now

@forki forki closed this Jun 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment