I received an email this AM from a fellow packager:
*Please, do not put source tarballs in git. This is not what is supposed to happen. Package sources are managed with fedpkg, they are just added to the lookaside cache and to the .gitignore file so you don't add them by mistake to the repository.https://fedoraproject.org/wiki/Package_maintenance_guide https://fedoraproject.org/wiki/Package_maintenance_guideHow did you manage to add them to the commit even if they are already in .gitignore?*
The package is megatools. The procedure I used was from the maintenance guide - yet apparently, these files ended up in a place where they weren't suppose to be.
The commands I used were:
- *Stage any small patches or new source files for commit:*
*git add (somefile) *
*Details *
*git does not consider all files in the working directory to be a part of the git repository by default (handy, for keeping other files around that are relevant, like the source tree). This tells git to start considering these files as part of the repository locally. When you 'commit' and 'push' later, this change is communicated to the server. *
- * Upload new source files to the lookaside cache:*
*fedpkg new-sources *
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball".
I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.
Artur Iwicki wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball".
I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.
Somewhat tangentially, it may be worth recommending that the .gitignore be edited suitably for the project as one of the first steps in maintainence.
By default 'fedpkg sources' will add the files being added to the lookaside cache to the .gitignore, but it will never remove older files, leading to an unnecessarily large .gitignore file.
It's much more legible to use git's pattern matching facilities to match the source files.
For example, the rpmlint's .gitignore contains the following¹:
/*.rpm /results_rpmlint/ /rpmlint-*/ /rpmlint-*.tar.gz
This keeps 'fedpkg source' from needing to add each new tarball to the file. More importantly, it makes it harder to accidentally add a tarball to git.
The additional rules are convenient for ignoring other files and directories which are often present in the work tree but are never intended to tbe committed.
Refer to gitignore(5) for details on the pattern matching rules.
¹ https://src.fedoraproject.org/rpms/rpmlint/blob/master/f/.gitignore
On Tue, Jul 24, 2018 at 01:32:45PM -0400, Todd Zullinger wrote:
Artur Iwicki wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball".
I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.
Somewhat tangentially, it may be worth recommending that the .gitignore be edited suitably for the project as one of the first steps in maintainence.
By default 'fedpkg sources' will add the files being added to the lookaside cache to the .gitignore, but it will never remove older files, leading to an unnecessarily large .gitignore file.
It probably isn't unreasonable to file an RFE against fedpkg to request more intelligent behaviour. eg given a tarball filename in majority of cases it is likely quite straightforward to identify the base name eg match the filename against something like this
(\w+)-(\d+(.\d+)*).(zip|tar.\w+|tgz)
And replace the 2nd part of the regex match with '*' to form the rule to add to the .gitignore file.
Regards, Daniel
On Tue, Jul 24, 2018 at 07:06:49PM +0100, Daniel P. Berrangé wrote:
On Tue, Jul 24, 2018 at 01:32:45PM -0400, Todd Zullinger wrote:
Artur Iwicki wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball".
I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.
Somewhat tangentially, it may be worth recommending that the .gitignore be edited suitably for the project as one of the first steps in maintainence.
By default 'fedpkg sources' will add the files being added to the lookaside cache to the .gitignore, but it will never remove older files, leading to an unnecessarily large .gitignore file.
It probably isn't unreasonable to file an RFE against fedpkg to request more intelligent behaviour. eg given a tarball filename in majority of cases it is likely quite straightforward to identify the base name eg match the filename against something like this
(\w+)-(\d+(.\d+)*).(zip|tar.\w+|tgz)
And replace the 2nd part of the regex match with '*' to form the rule to add to the .gitignore file.
Can't this be inferred from Sources: line? Just like rpmbuild does?
Daniel P. Berrangé wrote:
On Tue, Jul 24, 2018 at 01:32:45PM -0400, Todd Zullinger wrote:
Artur Iwicki wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball".
I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.
Somewhat tangentially, it may be worth recommending that the .gitignore be edited suitably for the project as one of the first steps in maintainence.
By default 'fedpkg sources' will add the files being added to the lookaside cache to the .gitignore, but it will never remove older files, leading to an unnecessarily large .gitignore file.
It probably isn't unreasonable to file an RFE against fedpkg to request more intelligent behaviour. eg given a tarball filename in majority of cases it is likely quite straightforward to identify the base name eg match the filename against something like this
(\w+)-(\d+(.\d+)*).(zip|tar.\w+|tgz)
And replace the 2nd part of the regex match with '*' to form the rule to add to the .gitignore file.
Yeah, perhaps someone will get bored and do that. Years ago, I submitted a patch to fedpkg/rpkg to have it resepct the existing .gitignore, such that if you have a pattern which matches the source being added, fedpkg won't add another entry.
Another option would be to have fedpkg use the package N-V-R to construct a gitignore entry and use that instead of the filename if the filename matches the pattern. I don't think it would be too hard to generate a pattern using a regex either, I just haven't been bothered enough to work on a patch.
2018-07-24 20:28 GMT+02:00 Todd Zullinger tmz@pobox.com:
Years ago, I submitted a patch to fedpkg/rpkg to have it resepct the existing .gitignore, such that if you have a pattern which matches the source being added, fedpkg won't add another entry.
My impression was that this is actually implemented in current fedpkg. I have /foo-*.tgz patterns in many of the packages I maintain, and fedpkg new-sources seems to respect that.
- Thomas
Thomas Moschny wrote:
2018-07-24 20:28 GMT+02:00 Todd Zullinger tmz@pobox.com:
Years ago, I submitted a patch to fedpkg/rpkg to have it resepct the existing .gitignore, such that if you have a pattern which matches the source being added, fedpkg won't add another entry.
My impression was that this is actually implemented in current fedpkg. I have /foo-*.tgz patterns in many of the packages I maintain, and fedpkg new-sources seems to respect that.
Yeah, it is. The patch I mentioned was applied (in rpkg¹). I could have stated that more clearly.
¹ f260d2e ("fedpkg: Try not to add redundant gitignore entries", 2010-09-08)
Todd Zullinger tmz@pobox.com wrote:
[…]
For example, the rpmlint's .gitignore contains the following¹:
/*.rpm /results_rpmlint/ /rpmlint-*/ /rpmlint-*.tar.gz
[…]
Apropos: Many .gitignores only reference the source files, i. e. not /results_${name} or /${name}-*.rpm. Therefore I usually add the latter two to .git/info/exclude and I wonder how others handle this.
Will fedpkg (and its backend) always create results_${name} and ${name}-*.rpm in the top directory or is the destination configurable? If the former, it would make sense to add them to .gitignore "for everybody", e. g. recommend that in the Packaging Guidelines. If not, I'd find it useful to have "fedpkg clone" add them to the initial .git/info/exclude.
Are there other approaches to have a clean "git status" dur- ing "normal" packaging development, tests, etc.?
Tim
Tim Landscheidt wrote:
Todd Zullinger tmz@pobox.com wrote:
[…]
For example, the rpmlint's .gitignore contains the following¹:
/*.rpm /results_rpmlint/ /rpmlint-*/ /rpmlint-*.tar.gz
[…]
Apropos: Many .gitignores only reference the source files, i. e. not /results_${name} or /${name}-*.rpm. Therefore I usually add the latter two to .git/info/exclude and I wonder how others handle this.
If it's a package I often work on, I generally add it to the .gitignore directly.
Will fedpkg (and its backend) always create results_${name} and ${name}-*.rpm in the top directory or is the destination configurable? If the former, it would make sense to add them to .gitignore "for everybody", e. g. recommend that in the Packaging Guidelines. If not, I'd find it useful to have "fedpkg clone" add them to the initial .git/info/exclude.
The results_* and *.src.rpm files are always created at the top level and are not configurable in fedpkg/rpkg (so far as I can tell).
I think it makes sense to have them in every package .gitignore file. Whether that's best done via the scripts which are used to create a new repo after a package review, via fedpkg/rpkg (automatically or by a command/option), or just documented as a best practice in the guidelines I'm not sure.
On 07/25/2018 11:59 PM, Todd Zullinger wrote:
Tim Landscheidt wrote:
Todd Zullinger tmz@pobox.com wrote:
[…]
For example, the rpmlint's .gitignore contains the following¹:
/*.rpm /results_rpmlint/ /rpmlint-*/ /rpmlint-*.tar.gz
[…]
Apropos: Many .gitignores only reference the source files, i. e. not /results_${name} or /${name}-*.rpm. Therefore I usually add the latter two to .git/info/exclude and I wonder how others handle this.
If it's a package I often work on, I generally add it to the .gitignore directly.
Will fedpkg (and its backend) always create results_${name} and ${name}-*.rpm in the top directory or is the destination configurable? If the former, it would make sense to add them to .gitignore "for everybody", e. g. recommend that in the Packaging Guidelines. If not, I'd find it useful to have "fedpkg clone" add them to the initial .git/info/exclude.
The results_* and *.src.rpm files are always created at the top level and are not configurable in fedpkg/rpkg (so far as I can tell).
I think it makes sense to have them in every package .gitignore file. Whether that's best done via the scripts which are used to create a new repo after a package review, via fedpkg/rpkg (automatically or by a command/option), or just documented as a best practice in the guidelines I'm not sure.
fedpkg also generates other directories and files from commands, e.g. prep or local.
fedpkg could write patterns to .git/info/exclude just after clone, which ignore known directories and files generated by its commands.
For other possible ignore patterns, they could be defined in user config file ~/.config/rpkg/fedpkg.conf that would be used in all packages that packager maintains.
To summarize, after package is cloned,
1. fedpkg writes known patterns to .git/info/exclude immediately 2. read predefined patterns from user config file, and if there is, write to .git/info/exclude as well.
Is this viable?
devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/...
On Tue, Jul 24, 2018 at 05:07:50PM -0000, Artur Iwicki wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources".
Actually "fedpkg new-sources" already does "git add" for you. I consider this an anti-feature (it "break" 'git diff', making 'git diff HEAD' or such necessary), but anyway, calling 'git add sources' is not necessary.
Zbyszek
On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki suve@fedoraproject.org wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources". I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball".
I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.
Thanks for the clarification and taking the initiative to get it fixed. As far as "poorly worded", that's a candidate for understatement of the day... LOL
It uses the exact same wording - "new source files" in both statements with absolutely no differentiation:
- Stage any small patches or new source files for commit: - Upload new source files to the lookaside cache:
Dne 24.7.2018 v 22:24 Gerald B. Cox napsal(a):
On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki <suve@fedoraproject.org mailto:suve@fedoraproject.org> wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources".
"new-sources" should not be mentioned in basic documentation. These should be added via means of "fedpkg import package.srpm".
The workflow for new package is then:
~~~ $ fedpkg import mypackage.srpm $ git commit -m "initial import" ~~~
and for updated package:
~~~ $ git add -u $ fedpkg clog $ git commit -F clog $ fedpkg import updated.srpm $ git commit --am ~~~
This way, the question if the file is stored in lookaside cache or git is done by fedpkg. Also please note that the "import" command removes patches which are not used any longer (not sure if it adds new patches ...).
V.
I believe what the guide means under "new source files" is e.g. when upstream does not provide an icon or a .desktop or an .appdata.xml file (or a systemd .service or whatever) and you add your own. This does not include "new upstream release tarball". I'll try to think of some way to make that more clear and submit a suggestion to change the guide text.Thanks for the clarification and taking the initiative to get it fixed. As far as "poorly worded", that's a candidate for understatement of the day... LOL
It uses the exact same wording - "new source files" in both statements with absolutely no differentiation:
- Stage any small patches or new source files for commit:
- Upload new source files to the lookaside cache:
devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-leave@lists.fedoraproject.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/...
On Wed, Jul 25, 2018 at 01:48:58PM +0200, Vít Ondruch wrote:
Dne 24.7.2018 v 22:24 Gerald B. Cox napsal(a):
On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki <suve@fedoraproject.org mailto:suve@fedoraproject.org> wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources"."new-sources" should not be mentioned in basic documentation. These should be added via means of "fedpkg import package.srpm".
The workflow for new package is then:
$ fedpkg import mypackage.srpm $ git commit -m "initial import"and for updated package:
$ git add -u $ fedpkg clog $ git commit -F clog $ fedpkg import updated.srpm $ git commit --am
Why make two separate commits here?
Zbyszek
Dne 25.7.2018 v 13:53 Zbigniew Jędrzejewski-Szmek napsal(a):
On Wed, Jul 25, 2018 at 01:48:58PM +0200, Vít Ondruch wrote:
Dne 24.7.2018 v 22:24 Gerald B. Cox napsal(a):
On Tue, Jul 24, 2018 at 10:07 AM, Artur Iwicki <suve@fedoraproject.org mailto:suve@fedoraproject.org> wrote:
That section of the guide is a bit poorly worded. You should *not* use "git add" on source tarballs. These should be added only via means of "fedpkg new-sources $FILES; git add ./sources"."new-sources" should not be mentioned in basic documentation. These should be added via means of "fedpkg import package.srpm".
The workflow for updated package:
$ git add -u $ fedpkg clog $ git commit -F clog $ fedpkg import updated.srpm $ git commit --amWhy make two separate commits here?
If I am not mistaken, you cannot import if your git repository is not in clean state, so you would have to reset the repository. Also, the files in the repository are in state I want to commit, but the SRPM might be a bit obsolete, typically due to last second changes of changelog. So the "commit" and later "commit --amend" is safe workflow IMO.
V.
On Wed, Jul 25, 2018 at 4:48 AM, Vít Ondruch vondruch@redhat.com wrote:
"new-sources" should not be mentioned in basic documentation. These should be added via means of "fedpkg import package.srpm".
The workflow for new package is then:
$ fedpkg import mypackage.srpm $ git commit -m "initial import"and for updated package:
$ git add -u $ fedpkg clog $ git commit -F clog $ fedpkg import updated.srpm $ git commit --amThis way, the question if the file is stored in lookaside cache or git is done by fedpkg. Also please note that the "import" command removes patches which are not used any longer (not sure if it adds new patches ...).
Thanks. If fedpkg has this capability IMO it should be referenced in the package maintenance guide. I understand that documentation is boring, but if someone makes a change or adds capability to a package, they need to take a few moments to update references to those capabilities in the documentation - especially if that documentation is something as important as the "package maintenance guide". I reference the guide every single time I make changes to packages - just to ensure I'm following the current rules. At least for me, it's vital that these instructions are current and correct.
-
Artur Iwicki -
Chenxiong Qi -
Daniel P. Berrangé -
Gerald B. Cox -
Thomas Moschny -
Tim Landscheidt -
Todd Zullinger -
Tomasz Torcz -
Vít Ondruch -
Zbigniew Jędrzejewski-Szmek