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 warning message for package folder (#4438) #4464

Merged
merged 6 commits into from Feb 7, 2019

Conversation

Projects
None yet
3 participants
@uilianries
Copy link
Member

commented Feb 4, 2019

Hi!

Do not display the warning "No files copied from build folder!" when actually there are files in the package folder. I've changed past tests to accept the new behavior, now some tests contain a file in the package folder, so it should not show any warning.

Add new test using cmake install instead of self.copy, the same scenario represented on issue #4438

Changelog: Fix: Do not display the warning when there are files in the package folder (#4438).
Docs: Omit

fixes #4438

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

#4438 Fix warning message for package folder
- Do not display the warning "No files copied from build folder!" when
  actually there are files in the package folder.
- Change past test to accept the new behavior.
- Add new test using cmake install instead of self.copy

Signed-off-by: Uilian Ries <uilianries@gmail.com>

@ghost ghost assigned uilianries Feb 4, 2019

@ghost ghost added the stage: review label Feb 4, 2019

@memsharded
Copy link
Contributor

left a comment

I think I would go one step further, and:

  • remove the conanfile.copy.report(package_output) mechanism from the package().
  • Wait for the warning message until the manifest is created.
  • Obtain the summary and possible warning about not package files from the manifest.

The same mechanism could be applied to the export and totally get rid of the report of FileCopier.

@uilianries

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2019

Obtain the summary and possible warning about not package files from the manifest.

Yeah, much better! I'll update this. Thanks

@lasote lasote added this to the 1.13 milestone Feb 5, 2019

#4438 Check package file by manifest
- Only warn about no files copied from build/package, when
  there is no files listed in the manifest. The file conaninfo.txt will
  ignore because it's generated by Conan.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
Show resolved Hide resolved conans/client/packager.py Outdated
#4438 Change output message to "packaged"
- Remove copier.report() from package()
- Change package() messages to fit better the stage

Signed-off-by: Uilian Ries <uilianries@gmail.com>
Show resolved Hide resolved conans/client/packager.py Outdated
Show resolved Hide resolved conans/client/packager.py Outdated

uilianries added some commits Feb 6, 2019

#4438 Remove the last copier from package. Good bye old friend
Signed-off-by: Uilian Ries <uilianries@gmail.com>
#4438 Fix unit tests
Signed-off-by: Uilian Ries <uilianries@gmail.com>
@memsharded
Copy link
Contributor

left a comment

Now it is very good, just have a look to the test that could be much faster avoiding CMake. Thanks!

Show resolved Hide resolved conans/test/functional/command/package_test.py Outdated
#4438 Remove slow cmake test
Signed-off-by: Uilian Ries <uilianries@gmail.com>

@memsharded memsharded merged commit ffd9ca1 into conan-io:develop Feb 7, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Feb 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.