-
Notifications
You must be signed in to change notification settings - Fork 121
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
Simplify packit custom create archive command #249
Conversation
@lachmanfrantisek, is there anything wrong with the config?
The full path to the archive is printed as the last line:
|
It works nicely locally. So there is some problem with the service. I'll create an issue. Sorry for this occurring problem. |
Potential problem:
(You can try using relative paths, but I still create an issue for this.) |
4151f72
to
9c820af
Compare
@lachmanfrantisek, thanks for looking into this. The relative path does not work:
I've tried both |
Thank you for testing all variants. We need to fix that on our side. There might a problem with the subdirectory. |
@lachmanfrantisek, thanks for looking into this. Let me know when I can test it again. |
/packit build |
@lachmanfrantisek, has there been any progress to make this working? |
Unfortunately no, sorry. But @TomasTomecek is now working on the shell expansion for commands and is having some problems that can be related to this. |
/packit build |
It seems that the archive is in |
@TomasTomecek, yes, that was the intention: To simplify Makefile so that moving the archive is not necessary. Providing the full/relative path to the archive in the output should be sufficient for Packit to find it. |
So you're saying that the build should pass with the current changes and the fact the SRPM can't be created is wrong? |
@TomasTomecek, that's what I've understood from our discussion with @lachmanfrantisek: That if the custom archive command prints out a valid path the to archive it should just work. Which makes sense to me. Otherwise users are forced to move the archive to proper location. Fixing this in one place (packit) seems to me to be a better option than duplicating this unnecessary action again and again in individual Makefiles. |
@TomasTomecek We are creating the symlink, but it does not work for some reason in the service:
|
I suspect the symlinks are not being propagated well b/w sandbox and worker. Anyway, I'm really confused here. I can see the tarball path being printed:
So how come that packit goes for We should probably add some debug prints in packit codebase whether it's able to find the archive. We'd also know if the symlinking works. |
I've looked around it a bit; enforced same handling of paths as in service.
@TomasTomecek That is because the build command is called with every directory set as Tried to break it in multiple ways:
|
Trying with |
Seems this did not help either. The resulting path was:
But:
|
Sorry for the confusion, I think it would be best to continue with Also would you like to try stage version (more info: packit/packit-service#649) of packit? It would help with catching problems early and speed up debugging of problems like this. :) |
Ok, I'll revert the change to use relative path. I've also enabled the staging instance. Let's see... |
Thanks. PR with extended logging is not merged yet, I'll trigger rebuild when it gets merged. |
/packit build |
Only users with write or admin permissions to the repository can trigger Packit-as-a-Service |
1 similar comment
Only users with write or admin permissions to the repository can trigger Packit-as-a-Service |
/packit build |
we should make @mfocko packit admin :) it seems the change you did in packit is not deployed to stg yet, let me retrigger stg worker image build |
/packit build |
Only users with write or admin permissions to the repository can trigger Packit-as-a-Service |
1 similar comment
Only users with write or admin permissions to the repository can trigger Packit-as-a-Service |
/packit build |
Wow, passed! are we good here? I just can't believe it that the fix was so "easy" |
Nice! Looks good :) |
@lachmanfrantisek, @TomasTomecek, @mfocko, works nice in production as well. Thanks for fixing this! |
@psss thank you for your patience and working with us! |
No description provided.