#1046 Improve separate test suite sourcing instructions
Closed 2 years ago by oturpe. Opened 3 years ago by oturpe.
oturpe/packaging-committee ruby-test-fetch  into  master

@@ -533,8 +533,7 @@ 

  

  ....

  # git clone https://github.com/bebanjo/delorean.git && cd delorean

- # git checkout v1.2.0

- # tar -czf rubygem-delorean-1.2.0-specs.tgz spec/

+ # git archive -o rubygem-delorean-1.2.0-specs.tgz v1.2.0 spec

  Source1: %{name}-%{version}-specs.tgz

  

  # ...

Instructions given for creating a source archive for test suites can be improved slightly. Existing method creates the archive from checked out Git branch. This leaves open the possibility that there are local changes in the working tree, which end up packaged. The proposed method avoids the possibility by never checking out anything and relying on 'git archive' that operates directly on committed items in the repository.

This proposal originates from discussion at jekyll-rubygem#4.

@pvalena and @vondruch could check if I got the rationale for this change right.

It might make the guidelines better, but the discussion actually originated in a PR for one of my packages, where this is making it objectively worse than what I did. -1

It might make the guidelines better, but the discussion actually originated in a PR for one of my packages, where this is making it objectively worse than what I did. -1

Could you explain why it is making things worse? Will the reason be valid for all packages? In that case, perhaps we should think about updating the guidelines to match the way this is done in your package.

git -C delorean archive -o $PWD/rubygem-delorean-1.2.0-specs.tgz spec maybe instead of 2 lines?

In rubygem-jekyll (and similar in other Ruby packages), I'm using this:

Source0:        https://rubygems.org/gems/%{gem_name}-%{version}.gem
Source1:        %{url}/archive/v%{version}/%{gem_name}-%{version}.tar.gz

Where the first one is the published gem, and the second one the archive from github. Both of them can be downloaded by spectool without manual clones and archiving steps. In other packages, I'm not only using the tests/ directory from a github tarball, but also other files that are missing from the published gem. So I think using the full GitHub tarball is definitely more versatile, and is actually automatiable, unlike the manual clone and archive step.

rebased onto cdd98bf173b63d8d05d6a32cd73aef4e5eba303a

3 years ago

I applied the suggestion by @ignatenkobrain , using less lines and command is better.

@decathorpe 's comment is more interesting. I think the point about pointing to upstream archive containing the needed resouces rather than listing instruction on how to create a suitable archive is very sound. Could it be that the existing example is quite bad actually? Perhaps the section should be adjusted to promote relying on upstream archives wherever possible, and only mention self-creating them as a last resort?

So I think using the full GitHub tarball is definitely more versatile, and is actually automatiable, unlike the manual clone and archive step.

While the versatility is true, the opposite of this is space for mistakes. If you have two copies of code, one from the .gem and the other from the github archive, there are higher chances you will work with the wrong one in the wrong time. Not mentioning that the full archive will very likely consume more space in look-a-side cache and what not.

git -C delorean archive -o $PWD/rubygem-delorean-1.2.0-specs.tgz spec maybe instead of 2 lines?

The --no-checkout, -C and $PWD are too much magic to my taste :/

rebased onto 51efdd9

3 years ago

git -C delorean archive -o $PWD/rubygem-delorean-1.2.0-specs.tgz spec maybe instead of 2 lines?

The --no-checkout, -C and $PWD are too much magic to my taste :/

Sure, there is some advanced git usage for a simple task. I pushed yet another formulation, which achieves both "no more than two lines" and "no advanced parameters".

git -C delorean archive -o $PWD/rubygem-delorean-1.2.0-specs.tgz spec maybe instead of 2 lines?

The --no-checkout, -C and $PWD are too much magic to my taste :/

Sure, there is some advanced git usage for a simple task. I pushed yet another formulation, which achieves both "no more than two lines" and "no advanced parameters".

Neat, thx :thumbsup:

So I think using the full GitHub tarball is definitely more versatile, and is actually automatiable, unlike the manual clone and archive step.

While the versatility is true, the opposite of this is space for mistakes. If you have two copies of code, one from the .gem and the other from the github archive, there are higher chances you will work with the wrong one in the wrong time. Not mentioning that the full archive will very likely consume more space in look-a-side cache and what not.

Yes, it is a tradeoff. A tradeoff of kind that I am unable to assess, simply because I do not have enough experience with all the things involved. So let me put it this way: I find it possible that all things considered, it would be better to suggest a different method that what is currently in the guidelines. But since I cannot judge it, I will retain the scope of this pull request as it is now, just a simple improvement to the current method. If anybody thinks the current method is not good, please open your own pull request.

In rubygem-jekyll (and similar in other Ruby packages), I'm using this:

Source0: https://rubygems.org/gems/%{gem_name}-%{version}.gem Source1: %{url}/archive/v%{version}/%{gem_name}-%{version}.tar.gz

Where the first one is the published gem, and the second one the archive from github. Both of them can be downloaded by spectool without manual clones and archiving steps. In other packages, I'm not only using the tests/ directory from a github tarball, but also other files that are missing from the published gem. So I think using the full GitHub tarball is definitely more versatile, and is actually automatiable, unlike the manual clone and archive step.

I actually have automation for creating the archive (used in my tooling, currently to create updates for rubygem-* packages). Not sure that helps though :).

Ping @tibbs @churchyard could you please review/merge?

Could we move the "cd ...." to the line with the actual archive creation?

Let's say I'm copy-pasting it, and the cloning fails (folder exists f.e.; with already pulled changes), then I run the command bellow- getting a failure, as I'm in a wrong directory.

Small change, but it helps :). (If we can't have -C ...)

Then always delete the repository and it will always work.

I mean, I either have the clone on my disk and I'm using different directory with cd so that line does not apply, or I don't have the repository at all and the the line applies as well.

There is so many workflow ... The current proposal is IMO helpful and concise

Then always delete the repository and it will always work.

Well it's like- why would I do that, if I can simply pull the changes? Do I really need to? If we need to do it, why not add it as an additional line?

I mean, I either have the clone on my disk and I'm using different directory with cd so that line does not apply, or I don't have the repository at all and the the line applies as well.

But those are all manual applications. TBH, I'm not aiming to change anyone's workflow- "anything works", instead I'm running the code using an automation to run the line for me. The automation will currently fail, as It'll be in a wrong directory. Yes, automated removal would fix that, but how am I to know which directory to remove? Second option would be to revert the repository to a pristine state after an upgrade (but that could be destructive, sometimes - and I try to avoid any such "workflow").

There is so many workflow ... The current proposal is IMO helpful and concise

Yes, but I'm also not requesting any big changes, just a really small one... which helps.

Automating the test suite retrieval would probably be a good idea, but I do not think the guidelines can be made to support that by changing the specfile comment example. According to the guidelines, these comments are not intended to have a script that can simply be run an expected to do the right thing. They are a simple memory aid to allow manual update: "Add the commands you used to get the tests into the specfile as comments. This will make it a lot easier the next time you will need to get them."

I will keep this pull request as it is. If you think the guidelines can be further improved to support better automation, you should open another pull request about it.

Right, let's make the automation smarter instead.

Tagging as meeting just to make sure this gets any consideration it needs.

Metadata Update from @tibbs:
- Pull-request tagged with: meeting

2 years ago

One downside of downloading git tarball is that there're (duplicitated) sources and various other resources in it, and therefore it can get quite big.

Not sure what the current state of this is, but I forgot to mention that I already have a well-tested automation[1] for creating the archive from the comments (used in my tooling, currently to create updates for rubygem-* packages). WRT guidelines, maybe these could have some "tag" / "prefix" so we know they're aimed at automation? But you're right, let's follow up on this in another issue.

[1] https://github.com/pvalena/theprototype/blob/main/pkgs/sources.sh

This pull request has been open for a long time already,
led to much more debate than I expected,
and is quite unimportant in the end.
I do not really have any interest in pursuing this,
so I will close this issue now.

Pull-Request has been closed by oturpe

2 years ago