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

Conan CLI: Add option to conan remove to just skip without error if package does not exist #16221

Closed
vincesp opened this issue May 8, 2024 · 13 comments
Assignees

Comments

@vincesp
Copy link

vincesp commented May 8, 2024

The conan remove command is missing an option to skip without returning an error if the package to be removed does not exist.

Background: It is really cumbersome to build an Azure pipeline that detects if a package exists and only runs the conan remove task if it does. If using a pool with numerous agents, we cannot be sure in which state the conan cache on every single agent is, so we need to clean the cache just to be sure. Adding continueOnError: true to the task only helps partially, as the pipeline run will show "completed with warnings" which does not reflect the actual outcome properly.

@memsharded memsharded self-assigned this May 8, 2024
@memsharded
Copy link
Member

Hi @vincesp

This doesn't look like a docs issue, but a client one. Moving it to the Conan client repo.

@memsharded memsharded transferred this issue from conan-io/docs May 8, 2024
@memsharded
Copy link
Member

memsharded commented May 8, 2024

The conan remove command is missing an option to skip without returning an error if the package to be removed does not exist.

$ conan remove zlib/1.4
Remove summary:
Local Cache

This works without failing, not sure what you mean (zlib/1.4 doesn't exist yet)

Background: It is really cumbersome to build an Azure pipeline that detects if a package exists and only runs the conan remove task if it does. If using a pool with numerous agents, we cannot be sure in which state the conan cache on every single agent is, so we need to clean the cache just to be sure.

It is not clear the background. Why would you remove a package if it exists? The whole purpose of using Conan is not having to rebuild something that already exists, this flow shouldn't be necessary in the first place.

@memsharded
Copy link
Member

Hi @vincesp

Any further feedback here?

@memsharded memsharded added the staled The issue has been inactive for a while and will be closed soon label Jul 26, 2024
@vincesp
Copy link
Author

vincesp commented Jul 26, 2024

@memsharded

In a build pipeline, we want to clean certain package to force their rebuild. But we don't want to empty the local cache completely, as this would defeat the purpose of a cache.

As we have many agents in our pool, we cannot be sure which packages have been built on which agent before.

@memsharded
Copy link
Member

In a build pipeline, we want to clean certain package to force their rebuild. But we don't want to empty the local cache completely, as this would defeat the purpose of a cache.

But rebuilding a package that exists in the server is considered a pipeline or model error. Anything that produces a new package revision for an existing recipe revision + package_id is a model error and leaves the door open for problems in traceability and reproducibility. Conan packages should only be built when there are changes in the original sources/recipe (captured in the recipe-revision), or when there are changes in the configuration (settings, options or dependencies changing, captured in the package_id). Conan will automatically build only what is strictly necessary, having to empty the local cache from certain packages to force a re-build sounds against the recommended practices.

It is also not clear what is the error:

λ conan remove potato/1.0
Remove summary:
Local Cache

works well, it doesn't fail, even if potato/1.0 doesn't exist in the cache.

@memsharded memsharded removed the staled The issue has been inactive for a while and will be closed soon label Jul 26, 2024
@vincesp
Copy link
Author

vincesp commented Jul 29, 2024

PS> conan --version     
Conan version 2.4.1
PS> conan remove -c zlib
ERROR: Recipe 'zlib' not found
PS> echo $LASTEXITCODE
1

So in our pipelines, we have the following code:

- script: conan remove -c ${{ parameters.packageName }} || true
  displayName: Conan Remove ${{ parameters.packageName }} (POSIX)
  condition: or(eq(variables['Agent.OS'], 'Linux'), eq(variables['Agent.OS'], 'Darwin'))

- script: conan remove -c ${{ parameters.packageName }} || exit /b 0
  displayName: Conan Remove ${{ parameters.packageName }} (Windows)
  condition: eq(variables['Agent.OS'], 'Windows_NT')

@memsharded
Copy link
Member

Ok, I see what you mean.

I think this is due to some legacy behavior that couldn't be fully removed in Conan 2, when using just a package name.

The recommended approach for this is using patterns:

conan remove "zlib/*" -c

will achieve the behavior that you want, that is, remove all zlib versions, but not erroring if it doesn't exist. Can you please try that and let me know?

@vincesp
Copy link
Author

vincesp commented Jul 29, 2024

It seems to work.

@memsharded
Copy link
Member

Ok, great, thanks for the feedback.

I have checked the docs, and I cannot find occurrences of this, but the conan remove "zlib/*" syntax is used, for example in https://docs.conan.io/2/reference/commands/remove.html.

I have also checked the code and the use cases. There might be users relying on this as a feature, for example if doing an operation (upload, remove) after some package has been created, so it is guaranteed to be there, and if it is not there, they are happy to get an error. So most likely it is not worth the risk to try to change this behavior and make it not raise an error for the usage without a pattern *.

I am closing this as solved, no option is necessary as the pattern syntax achieve that, but please re-open or create new tickets for any further question or issue. Thanks for the feedback!

@vincesp
Copy link
Author

vincesp commented Jul 30, 2024

Update:

Even conan remove -c "*" does not cleanup the local cache completely.

In order for our pipelines to use the information from Artifactory, not from previous (potentially failed) local build attempts, we had to add the following to our pipelines:

- script: |
    rm -rf $HOME/.conan2/p
    rm $HOME/.conan2/.conan.db
  displayName: Clean Conan cache (POSIX)
  condition: or(eq(variables['Agent.OS'], 'Linux'), eq(variables['Agent.OS'], 'Darwin'))

- script: |
    rmdir /s /q %USERPROFILE%\.conan2\p
    del %USERPROFILE%\.conan2\.conan.db
  displayName: Clean Conan cache (Windows_NT)
  condition: eq(variables['Agent.OS'], 'Windows_NT')

@memsharded
Copy link
Member

conan remove -c "*"

This removes all the packages from the cache, it won't remove configuration files or other temporary files.

In order for our pipelines to use the information from Artifactory, not from previous (potentially failed) local build attempts, we had to add the following to our pipelines:

Local build attempts in the Conan cache that didn't succeed are unreachable, they are in folders generated with a uuid.uuid4(), so it is not possible that any posterior conan install or similar operation will use them accidentally.

In any case, for CI it is recommended that every job will use a new, blank CONAN_HOME, because the Conan cache is not concurrent, so if you plan to allow CI jobs to run concurrently in the same machine, under the same user, then you will need to make every CI job to run in its one Conan home setting a different new CONAN_HOME per job.

@vincesp
Copy link
Author

vincesp commented Jul 31, 2024

We regularly run into a situation where the build agent reports "can't find a 'xyz/1.2.3' package binary 'hash' for the configuration: …" while the exact hash is indeed available on our Artifactory server, but under a different parent hash. If conan already has the first part of the hash available locally from a previous failed build attempt (hence with an incomplete set of configurations), then it won't go back to the server and check. conan remove -c "*" won't fix that, neither running create --update, only deleting the files as above will.

@memsharded
Copy link
Member

The "parent hash" is the recipe revision, it is associated with recipe and source changes.
The package binary hash is the package_id, associated with the configuration (profile, settings, options, etc)

conan remove -c "*" won't fix that

I am not sure about this. The conan remove -c "*" completely remove all existing revisions from the local cache, so it won't resolve any previous attempt from the local cache. If it does, then we would need a reproducible example in a new ticket to investigate it, it would be a bug.

A different story is if it is resolving against a server repo that got uploaded a new recipe-revision with "incomplete binaries", lets try to clarify this case just in case:
Everytime a new recipe revision is created, it needs to build new binaries, as it is new source.
The way this can be handled is using 2 repositories and promotions:

  • Create a "build" server repo besides your normal server repo, lets call it "develop"
  • When a new recipe revision is available to build new packages (typically like an open pull request), then create packages for the different configurations.
  • Upload the packages with the different configurations to the "build" repo
  • Only when all configurations are available and correct, the pull request can be merged, and the packages promoted
  • The promotion of packages is a copy from all the built packages from the "build" repo to the "develop" repo. Recall that with package deduplication, the server is not taking any extra storage space for this operation, which is really fast, independently of the package size.

This way it is impossible to have an incomplete set of configurations in the server repo.
You might want to have a look to the ongoing work in conan-io/docs#3799
In any case, this about server side packages, would also seem a very different conversation from the original one, if you want to ask any further question it might be better to create a new ticket. Thanks!

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