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

[feature] Check original filename when pulling from sources backup #17782

Open
1 task done
SoShiny opened this issue Feb 14, 2025 · 2 comments
Open
1 task done

[feature] Check original filename when pulling from sources backup #17782

SoShiny opened this issue Feb 14, 2025 · 2 comments
Assignees

Comments

@SoShiny
Copy link

SoShiny commented Feb 14, 2025

What is your suggestion?

Feature request

Check the filename listed in the conandata.yml (part of the URL) against the filename listed in the backup summary <sha>.json, and either raise a warning or an error, depending on some configuration option.

Context

We are using a generic package registry on our JFrog Artifactory for backing up Conan package sources. Generally, it's a very nice feature.

I had a very puzzling issue building a new version of one of our Conan packages, which I ultimately tracked down to the following:

  1. In the conandata.yml, I mistakenly added the sha256 sum of the old version's source archive as the sha of the new version's source archive
  2. In the global.conf we have the backup server first for faster downloads and ensured availability (core.sources:download_urls=['thebackupserver', 'origin'])
  3. When building the new version, the sha was used to get the source archive of the old version and no warning or error was raised
  4. The package just differs slightly from one to another version, so the build process did not fail. So we ended up with a "new" version of a package which behaved like the old version, because it was.

While 1) is clearly a user error, it is a very easy one to make when updating a version with minimal changes, because one typically just copies and alters the entry which was there before. The choice of 2) should also be relatively common.

The backup sources also include the summary <sha>.json file which contains the original source URL which includes the archive name.

Minimal reproducible example

The attached minimal_example.tar.gz builds a package my_package which is just a C program which prints a version number. It contains two source archives for two versions. The conandata.yml contains the mistake of version 2.0.0 having the same sha as 1.0.0.

Steps to reproduce issue

  1. Tested with Conan versions 2.12.2 and 2.8.0

  2. Set global.conf option, such that the backup server is preferred (core.sources:download_urls=['thebackupserver', 'origin'])

  3. Go to the recipe folder and run conan create . --version 1.0.0

    1. This will build the executable, run the test, and print Version 1.0.0
  4. Upload the package and it's backup sources conan upload my_package/1.0.0:* -r <remote>

    1. This should have the following log:
    -------- Uploading backup sources --------
    Uploading summary '3cc2cb352456047985fce2774e120b4f905b487c4ebd6ee6fb98e6aeb728bf6a.json' to backup sources server
    Uploading file '3cc2cb352456047985fce2774e120b4f905b487c4ebd6ee6fb98e6aeb728bf6a' to backup sources server
    Upload backup sources complete
    
  5. Now, build the second version conan build . --version 2.0.0

    1. You'll see that the test has the unexpected output
    ======== Testing the package: Executing test ========
    my_package/2.0.0 (test package): Running test()
    my_package/2.0.0 (test package): RUN: my_package
    Version 1.0.0
    
    1. You'll also see that the source archive was "found" in the backup
    ======== Installing packages ========
    my_package/2.0.0: Calling source() in /.../.conan2/p/my_paf3d6b04c4078f/s
    my_package/2.0.0: Sources for file:///../../my_package_v2.tar.gz found in remote backup https://..../
    

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@AbrilRBS AbrilRBS self-assigned this Feb 14, 2025
@AbrilRBS
Copy link
Member

Hi @SoShiny thanks a lot for the report. You're right that this is a potential issue and it should be checked. In ConanCenterIndex we found the same situation and now we have linter rule to check for those, but I agree that something should be done client-side

Im proposing #17813 to address this, but no guarantees that it will make it as-is, as we need to check that it covers just these cases, with the idea being that you can enable warnings as errors with core:warnings_as_errors for risk warnings and ensure that Conan fails if it finds this issue - Note that it can't be made an error by default, as some recipes my want to re-download some sources such as licenses that are common to more than one version for example

@SoShiny
Copy link
Author

SoShiny commented Feb 22, 2025

Thanks for looking into it. In the meantime, could you point me to how I could do the same kind of linting on our recipes?

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

No branches or pull requests

2 participants