Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-15 Thread Florian Festi
Merged as #2911
Thanks for the patch!

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1945539722
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-15 Thread Florian Festi
Closed #2734.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#event-11811695188
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-14 Thread Panu Matilainen
@pmatilai commented on this pull request.



>  if (rpmGlob(attrPath, NULL, ) == 0) {
-   nattrs = argvCount(files);
-   fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
-   for (int i = 0; i < nattrs; i++) {
-   char *bn = basename(files[i]);
-   bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
-   }
-   fc->atypes[nattrs] = NULL;
-   argvFree(files);
+   nfiles = argvCount(files);
+}
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   argvAddUniq(_attrs, bn);

I realised the really right way to do this is to refactor the discovery/init 
split to a separate commit, and then the local_attr thing doesn't need to 
change anything at all, only add itself.
Having asked this to be one commit already I couldn't very well ask you to do 
this, so here goes: https://github.com/rpm-software-management/rpm/pull/2911

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#discussion_r1490545272
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-14 Thread Florian Festi
@ffesti pushed 1 commit.

805d83186bec6297ef35a1b0d1405aec845c069a  Add %_local_file_attrs macro

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/95b89b63ab47a576e1006e512cb14935d980361e..805d83186bec6297ef35a1b0d1405aec845c069a
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-14 Thread Florian Festi
@ffesti commented on this pull request.



>  if (rpmGlob(attrPath, NULL, ) == 0) {
-   nattrs = argvCount(files);
-   fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
-   for (int i = 0; i < nattrs; i++) {
-   char *bn = basename(files[i]);
-   bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
-   }
-   fc->atypes[nattrs] = NULL;
-   argvFree(files);
+   nfiles = argvCount(files);
+}
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   argvAddUniq(_attrs, bn);

Sure. There also is a "squash and merge" button somewhere  down there...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#discussion_r1489096578
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-14 Thread Panu Matilainen
@pmatilai commented on this pull request.



>  if (rpmGlob(attrPath, NULL, ) == 0) {
-   nattrs = argvCount(files);
-   fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
-   for (int i = 0; i < nattrs; i++) {
-   char *bn = basename(files[i]);
-   bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
-   }
-   fc->atypes[nattrs] = NULL;
-   argvFree(files);
+   nfiles = argvCount(files);
+}
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   argvAddUniq(_attrs, bn);

Doh, I was somehow thinking rpmGlob() returned basenames there but, but (of 
course) it doesn't. So scratch that part, sorry.

The location is configurable but it's still always just a single directory, it 
cannot possibly have multiple files by the same name. It's not that 
argvAddUniq() is *wrong*, it just gives the reader the wrong impression 
basically. Similarly moving this .attr splitting loop out of the rpmGlob() 
if-block seems strange, because there's only ever anything to do on rpmGlob() 
success. So that's kinda backwards to what you say about not wanting to think 
about the case where it doesn't find anything because that's already handled, 
and just obfuscates the diff for no good reason. It's always better to let the 
actual change stand out in the diff when reasonably possible.

This is the *actual* change to the first part here. nattr gets recalculated 
later on, but most of the time it's already correct from here:

```
 /* Discover known attributes from pathnames + initialize them */
 if (rpmGlob(attrPath, NULL, ) == 0) {
nattrs = argvCount(files);
-   fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
for (int i = 0; i < nattrs; i++) {
char *bn = basename(files[i]);
bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
+   argvAdd(_attrs, bn);
}
-   fc->atypes[nattrs] = NULL;
argvFree(files);
 }
+
+/* Get file attributes from _local_file_attrs macro */
+/* ... /

```
(actually one could just pass  to rpmGlob() and avoid the argvCount(), 
but that too would kinda obfuscate the diff)


Patch cleanliness aside, this all needs to be just one commit. It's a single 
new feature with its tests, we are not interested in PR review in the commit 
history.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#discussion_r1489081084
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
@ffesti commented on this pull request.



>  if (rpmGlob(attrPath, NULL, ) == 0) {
-   nattrs = argvCount(files);
-   fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
-   for (int i = 0; i < nattrs; i++) {
-   char *bn = basename(files[i]);
-   bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
-   }
-   fc->atypes[nattrs] = NULL;
-   argvFree(files);
+   nfiles = argvCount(files);
+}
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   argvAddUniq(_attrs, bn);

Yes, but they are file names and not just attribute names. An while they are 
unique because we hard coded the glob, making the location configurable may 
turn up the same names from different locations. That's why I made the code a 
bit more defensive than it needs to be. Also I don't even want to think what 
rpmGlob returns if it doesn't find anything. Overall I don't think this is 
going to get that much simpler in the end.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#discussion_r1488176745
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Panu Matilainen
@pmatilai commented on this pull request.



>  if (rpmGlob(attrPath, NULL, ) == 0) {
-   nattrs = argvCount(files);
-   fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
-   for (int i = 0; i < nattrs; i++) {
-   char *bn = basename(files[i]);
-   bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
-   }
-   fc->atypes[nattrs] = NULL;
-   argvFree(files);
+   nfiles = argvCount(files);
+}
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   argvAddUniq(_attrs, bn);

The files coming out of rpmGlob() are unique already, you don't need 
argvAddUniq() here. What I meant is that you could simply use the argv coming 
from rpmGlob() and argvAddUniq() the local stuff to that.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1878019777
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
OK, removed one underscore from the macro name, rewrite the init code and added 
two test cases that deal with already installed file attributes.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1941456054
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
@ffesti pushed 1 commit.

e1dea8eafaf3f4da91e7ba132f9e953eec3a9665  Add more test cases

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/7c64e73308a328d3aad40c118024f799503bc96f..e1dea8eafaf3f4da91e7ba132f9e953eec3a9665
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
@ffesti pushed 1 commit.

7c64e73308a328d3aad40c118024f799503bc96f  Local File Attrs: Remove one 
underscore

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/ab3a293498ec59129d3551e76e444e964b9f0985..7c64e73308a328d3aad40c118024f799503bc96f
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
@ffesti pushed 1 commit.

7023620eb258af338b1e53b3806b8f66ad9384d7  Local File Attrs: Remove one 
underscore

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/fb9b6ec25e2e72daa7944175e64c2928104cd016..7023620eb258af338b1e53b3806b8f66ad9384d7
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
@ffesti pushed 1 commit.

fb9b6ec25e2e72daa7944175e64c2928104cd016  Local File Attrs: Remove one 
underscore

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/5ff3074187b888f9ff62416d9495fe36f7890468..fb9b6ec25e2e72daa7944175e64c2928104cd016
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-13 Thread Florian Festi
> One more thing wrt the macro name: I wonder if this is a case where it should 
> _not_ have those leading underscores. The generator itself is full of double 
> underscore names, but the newly added macro here is something directly 
> intended for packager use in a spec. I dunno, it may even be more confusing 
> to have one of the macros without them 

May be someone(tm) should write down the policy regarding underscores. May be 
in a [RPM Design Philosophy](#2897) document...
IIRC the idea was that macros without underscore are the packagers. One 
underscore are RPM's macro to be used by packagers and double underscore are 
internal macros to be left untouched. Not that this really is done that way.

But I agree that there should probably be less underscores.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1941125050
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Vít Ondruch
@voxik commented on this pull request.



> @@ -995,6 +995,25 @@ runroot rpm -qp --requires 
> /build/RPMS/noarch/shebang-0.1-1.noarch.rpm|grep -v ^
 [])
 RPMTEST_CLEANUP
 
+AT_SETUP([Local dependency generator])
+AT_KEYWORDS([build])
+RPMTEST_CHECK([
+RPMDB_INIT
+
+runroot rpmbuild -bb --quiet \
+   --define '__local_file_attrs my_test_attr' \

If this covered multiple generators to demonstrate that something like this is 
supported, that would be even better

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1865093933
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Panu Matilainen
One more thing wrt the macro name: I wonder if this is a case where it should 
*not* have those leading underscores. The generator itself is full of those, 
but the newly added macro here is something directly intended for packager use.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1929405020
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Panu Matilainen
@pmatilai commented on this pull request.



> @@ -995,6 +995,25 @@ runroot rpm -qp --requires 
> /build/RPMS/noarch/shebang-0.1-1.noarch.rpm|grep -v ^
 [])
 RPMTEST_CLEANUP
 
+AT_SETUP([Local dependency generator])
+AT_KEYWORDS([build])
+RPMTEST_CHECK([
+RPMDB_INIT
+
+runroot rpmbuild -bb --quiet \
+   --define '__local_file_attrs my_test_attr' \
+   --define '__my_test_attr_provides() foo(%{basename:%{1}})' \
+   --define '__my_test_attr_path .*' \

This covers on of the use-cases, but the excess free() in the code kinda proves 
that we need a test for the other scenario too where there's a pre-existing 
attr by the same name.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1865004852
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Panu Matilainen
@pmatilai commented on this pull request.



> - bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
+   nfiles = argvCount(files);
+}
+char * local_attr_names = rpmExpand("%{?__local_file_attrs}", NULL);
+ARGV_t local_attrs = argvSplitString(local_attr_names, ":", 
ARGV_SKIPEMPTY);
+nlocals = argvCount(local_attrs);
+fc->atypes = xcalloc(nfiles + nlocals + 1, sizeof(*fc->atypes));
+
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   // skip file attrs found in __local_file_attrs for now
+   for (j=0; (j < nlocals) && strcmp(bn, local_attrs[j]); j++);
+   if (j < nlocals) {
+   free(bn);

Might be more readable to just toss all basenames into a new argv with 
argvAddUniq() , which frees us from having to care about these details here.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#discussion_r1479661477
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Panu Matilainen
@pmatilai commented on this pull request.



> - fc->atypes = xcalloc(nattrs + 1, sizeof(*fc->atypes));
-   for (int i = 0; i < nattrs; i++) {
-   char *bn = basename(files[i]);
-   bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
+   nfiles = argvCount(files);
+}
+char * local_attr_names = rpmExpand("%{?__local_file_attrs}", NULL);
+ARGV_t local_attrs = argvSplitString(local_attr_names, ":", 
ARGV_SKIPEMPTY);
+nlocals = argvCount(local_attrs);
+fc->atypes = xcalloc(nfiles + nlocals + 1, sizeof(*fc->atypes));
+
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   // skip file attrs found in __local_file_attrs for now

/* */ comments, thanks.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1864948185
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Panu Matilainen
@pmatilai commented on this pull request.



> - bn[strlen(bn)-strlen(".attr")] = '\0';
-   fc->atypes[i] = rpmfcAttrNew(bn);
+   nfiles = argvCount(files);
+}
+char * local_attr_names = rpmExpand("%{?__local_file_attrs}", NULL);
+ARGV_t local_attrs = argvSplitString(local_attr_names, ":", 
ARGV_SKIPEMPTY);
+nlocals = argvCount(local_attrs);
+fc->atypes = xcalloc(nfiles + nlocals + 1, sizeof(*fc->atypes));
+
+for (int i = 0; i < nfiles; i++) {
+   char *bn = basename(files[i]);
+   bn[strlen(bn)-strlen(".attr")] = '\0';
+   // skip file attrs found in __local_file_attrs for now
+   for (j=0; (j < nlocals) && strcmp(bn, local_attrs[j]); j++);
+   if (j < nlocals) {
+   free(bn);

This is not right, basename() doesn't allocate, so this would free some random 
pointer inside the one that is.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1864947811
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Florian Festi
Ok, renamed back to `__local_file_attrs`. I squashed the patches and improved 
the docs a little bit. From my POV this is now complete. Can someone please 
prove read the docs? Thanks!

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1929219489
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-06 Thread Florian Festi
@ffesti pushed 1 commit.

5ff3074187b888f9ff62416d9495fe36f7890468  Add %__local_file_attrs macro

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/57334a2b0b0ad7d84e8e398bf6c6e6a8b53d2481..5ff3074187b888f9ff62416d9495fe36f7890468
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-02-02 Thread Panu Matilainen
> This all makes `__local_file_attrs` look less horrible than I first thought 

Indeed. It's sufficiently vague to cover both cases we care about, and totally 
accurate in the sense that its local to the build.

> `__additional_file_attrs` is actually kinda close but not quite as it implies 
> that it does not contains file attrs that are already installed. What about 
> `__add_file_attrs`? It's shorter and does not emphasis as much the separation 
> betwenn new and already existing.

I can live with either local or add. There are bigger crisis in the world 
:laughing: 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1923549189
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-31 Thread Vít Ondruch
`__embedded_file_attrs`?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1918940894
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-31 Thread Florian Festi
I think the issue here that they can be both overriding existing ones or being 
an add-on. This makes it difficult to find a good name that covers both cases. 
OK, technically the macro doesn't override existing file attrs. 
Overriding/defining the macros does. So here we are registering file attr 
names...

This all makes `__local_file_attrs` look less horrible than I first thought 
`__additional_file_attrs` is actually kinda close but not quite as it implies 
that it does not contains file attrs that are already installed. What about 
`__add_file_attrs`? It's shorter and does not emphasis as much the separation 
betwenn new and already existing.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1918928307
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-23 Thread Vít Ondruch
`__override_file_attrs`, which would give higher prominence to the regular 
installed attrs and discouraged use of this feature. But that brings me back to 
the `__extra_file_attrs`, which I have read after I was thinking about 
`__override_file_attrs` ;) IOW `__extra_file_attrs` sounds good to me.

`__additional_file_attrs` could also be option. It sounds mundane, but maybe 
that makes it the better choice.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1905700754
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-23 Thread Panu Matilainen
I find `packaged` quite misleading, because the case is to allow for attributes 
and generators that are local to the *build*. Those attributes *may also* be 
packaged, but that's not relevant for this feature. I see two separate main 
cases for this:
- build a package with the generator it ships to avoid having to build twice
- build a package with a generator that is only relevant for that package 

Back to bike-shedding. `__build_file_attrs` is closer maybe but then it 
suggests those are the *only* attrs for the build. `__extra_file_attrs` would 
seem accurate because no matter where they come from they are extra to the 
regular attrs.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1905503860
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-22 Thread Vít Ondruch
`packaged` or `package`? I am asking, because the generators are not packaged 
yet as I perceive that. But I can live also with `packaged` :)

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1904388907
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-22 Thread Vít Ondruch
@voxik commented on this pull request.



> @@ -132,6 +132,14 @@ Enabling the multifile mode is done by setting
 %__foo_protocol multifile
 ```
 
+## Using File Attributes in their own Package
+
+Normally file attributes and their dependency generators are shipped in 
separate packages that need to be installed before the package making use of 
them can be build.
+
+Since rpm 4.20 the names of file attribute shipped with the package can be put 
into the *__packaged_file_attrs* macro separated by colons (:). The macros that 
normally go into the *\*.attr* files still need to be defined (the dependency 
generators typically pointing to some Source files or some files in the install 
root).
+

I think that example would be useful here. Also the following paragraph is not 
very tangible without example.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1836891196
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-22 Thread Florian Festi
OK, I replaced the word "local" as its meaning is just too vague here. 
"Packaged" discourages using file attributes that are just on the machine 
without being shipped or being installed from another package. While this is 
technically possible this is something we clearly don't want to encourage.

These patches still need to be squashed and a combined commit message be added. 
I'll leave them separate for now to make it easier to review the last few 
changes on their own. 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1903994707
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-22 Thread Florian Festi
@ffesti pushed 2 commits.

2468b76d5b7e4bae8c55ddabb188d0d2f4807dbf  Filter duplicate file attributes
57334a2b0b0ad7d84e8e398bf6c6e6a8b53d2481  Add documentation for 
__packaged_file_attrs

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/5059816af9185cc7c3f19ad729b4dc657f912cde..57334a2b0b0ad7d84e8e398bf6c6e6a8b53d2481
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Florian Festi
OK, just to write down how the current system works:

The `fileattrs/*.attr` are read in at the beginning with all other macro files. 
The macros in there can be over written by the spec file - or by `%load` ing 
the `.attr` file in the Sources.

RPM goes through the list of file attrs (currently from globbing the `fileattr` 
directory) and stuffs the macros it can find into it's own data structure 
before evaluating them and running the dependency generators.

The has the disadvantage that when e.g. a `_provides` script is declared in the 
installed `.attr` file it will be excuted unless the macro is `#undefined` in 
the spec file. That's kinda unfortunate but something packagers probably can 
deal with. This also means that if the same name is used in 
`%__packaged_file_attrs` the over written macro values will also overwrite the 
installed settings as the macros are in the same name space. So the old scripts 
would not be used - only the new ones executed twice. But we can simply filter 
out the duplicates. Having the file attribute twice doesn't ever make sense as 
there is only one setting for it.




-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898627307
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Florian Festi
@ffesti pushed 1 commit.

5059816af9185cc7c3f19ad729b4dc657f912cde  Rename __local_file_attrs to 
__packaged_file_attrs

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/cd4bd81ad1c446c119a27eec77b2828b5ea1624a..5059816af9185cc7c3f19ad729b4dc657f912cde
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Vít Ondruch
> I think the issues is reproducibility and correctness. If you fix the 
> dependency generator in your package you don't want to old - possibly broken 
> - deps still in your package, just because the old package is still on your 
> build system.

That is actually good point.

So in the Ruby case, it would actually make sense to override the `rubygems` 
macros, which would make sure the generator shipped with Ruby is the only one 
used. IOW there would need to be:

~~~
%global __local_file_attrs rubygems

%global __rubygems_requires make -C 
%{_builddir}/%{buildsubdir}/%{_vpath_builddir} -s runruby 
TESTRUN_SCRIPT="--enable-gems %{SOURCE9}" 
%global __rubygems_provides make -C 
%{_builddir}/%{buildsubdir}/%{_vpath_builddir} -s runruby 
TESTRUN_SCRIPT="--enable-gems %{SOURCE10}"
%global __rubygems_conflicts make -C 
%{_builddir}/%{buildsubdir}/%{_vpath_builddir} -s runruby 
TESTRUN_SCRIPT="--enable-gems %{SOURCE11}"   
%global __rubygems_path ^%{gem_dir}/specifications/.*\.gemspec$
~~~

Actually, this leads me to another silly idea. Maybe the `rubygems.attr` could 
be loaded instead of the overrides if it was crafted properly ...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898330857
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Florian Festi
I think the issues is reproducibility and correctness. If you fix the 
dependency generator in your package you don't want to old  - possibly broken - 
deps still in your package, just because the old package is still on your build 
system.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898223470
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Vít Ondruch
> I am thinking about actually shipping the dependency generator but also using 
> it in the package itself.

This ^^ actually is my use case and here is how it looks in practice:

https://src.fedoraproject.org/rpms/ruby/blob/308b2c0ab2c6268847ed3bf2008e74bce334e810/f/ruby.spec#_187-190
https://src.fedoraproject.org/rpms/ruby/blob/308b2c0ab2c6268847ed3bf2008e74bce334e810/f/ruby.spec#_203-206
https://src.fedoraproject.org/rpms/ruby/blob/308b2c0ab2c6268847ed3bf2008e74bce334e810/f/ruby.spec#_795-800
https://src.fedoraproject.org/rpms/ruby/blob/308b2c0ab2c6268847ed3bf2008e74bce334e810/f/ruby.spec#_1330-1335

Saying all these, it makes me realize that in theory, if the `rubygems-devel` 
package was installed during Ruby build, the generators could actually run 
twice. But they are executed for specific path based on specific macro:

https://src.fedoraproject.org/rpms/ruby/blob/308b2c0ab2c6268847ed3bf2008e74bce334e810/f/rubygems.attr#_6

Not sure. In reality, having Ruby installed during build of Ruby is problematic 
for different reasons, so I don't have to be worried in this case.

> For that you need to make sure that it is only run once during the build - 
> even if the package is actually installed in the system building the package.

Do I? I don't think there is currently anything, which would prevent one 
generator running twice. E.g. if there are multiple `.attr` files pointing to 
the same script. Also the macros can be changed any time to point to the same 
script. And the generators can also be disabled by changing the macro.

Actually, from this POV, my original PR making the `local_generator` special 
kind of helps to mitigate the concerns. The more flexibility will provide more 
opportunities for clashes.

Nevertheless, let me be clear that I am not really arguing for or against of 
any of those variant. Both versions are acceptable. But it is good to 
understand all the corner cases 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898123753
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Florian Festi
Hmm, I guess we are taking about two different use cases. You are thinking 
about a dependency generator that is in the package only and not installed 
ever. For that you want a unique name that won't ever clash with installed file 
attributes.

I am thinking about actually shipping the dependency generator but also using 
it in the package itself. For that you need to make sure that it is only run 
once during the build - even if the package is actually installed in the system 
building the package. The you'd want the names to be the same and RPM should 
overwrite the installed one with the one in the package.

I guess we do want both use cases to work.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898072680
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Vít Ondruch
On the second thought, maybe it really is the right way. I am coming from place 
where no "local" generator us was possible, so one was already great 
improvement. But maybe there are use cases for multiple generators?

OTOH, one could call them via some helper script or by chaining them (in the 
worst case by executing `bash` or what not).

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898067418
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Vít Ondruch
@voxik commented on this pull request.



> @@ -866,6 +866,7 @@ RPMTEST_CHECK([
 RPMDB_INIT
 
 runroot rpmbuild -bb --quiet \
+   --define '__local_file_attrs local_generator' \

Maybe something like `totally_random_generator_name`

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#discussion_r1457125066
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Vít Ondruch
> OK, just hard coding one file attribute doesn't seem like a good idea. I 
> added a macro that allows you to register an arbitrary number of local file 
> attributes and generators.

Nice, now we can argue if the `__local_file_attrs` is the right name or rather 
e.g. `__package_local_file_attrs` 藍 Sorry, couldn't resist.

Nevertheless, thx for contributing. I see your point. OTOH, if I read your 
commit correctly, my original proposal had the advantage, where one could count 
with convention over configuration. IOW with your changes one additional line 
is needed to make this work.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1898047160
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-18 Thread Vít Ondruch
@voxik commented on this pull request.



> @@ -866,6 +866,7 @@ RPMTEST_CHECK([
 RPMDB_INIT
 
 runroot rpmbuild -bb --quiet \
+   --define '__local_file_attrs local_generator' \

While this is "just test case", maybe the `local_generator` could use different 
name, just to avoid the impression that the `local` is somehow mandatory in the 
latter

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1829143312
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-17 Thread Florian Festi
OK, just hard coding one file attribute doesn't seem like a good idea. I added 
a macro that allows you to register an arbitrary number of local file 
attributes and generators. I am still wondering if we need a mechanism to avoid 
executing generators twice when the package is already installed while a new 
version is building. May be we should use the original name and over write the 
locally installed configuration. This would be pretty simple to do I guess by 
just checking the list of previously loaded file attributes.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1895983933
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2024-01-17 Thread Florian Festi
@ffesti pushed 1 commit.

cd4bd81ad1c446c119a27eec77b2828b5ea1624a  Add %__local_file_attrs macro

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/61bd40a9df5170da6182e560d172fb16f4e3213b..cd4bd81ad1c446c119a27eec77b2828b5ea1624a
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-27 Thread Vít Ondruch
> If `local_generator.attr` file exists then `local_generator` created twice.

This is good point. Not sure if this is real problem though.

> Why not simply create an empty `local_generator.attr` file instead?

I have proposed this earlier in 
https://github.com/rpm-software-management/rpm/issues/782#issuecomment-1747200612
 and put this idea into practice for 
[Fedora](https://src.fedoraproject.org/rpms/rpm-local-generator-support). I 
have no preference.

I just wanted to move this forward a bit ;) now I stand by waiting for guidance 
(i.e. the right implementation and the acceptable name).



-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1782567947
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-26 Thread Panu Matilainen
I'd prefer a name that indicates the spec/package specifity somehow. @ffesti , 
@dmnks , thoughts, other ideas?

And yeah adding that empty file with just a comment on the purpose would 
actually be even more simple.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1780526290
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Mikolaj Izdebski
If `local_generator.attr` file exists then `local_generator` created twice. Why 
not simply create an empty `local_generator.attr` file instead?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1779258988
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Vít Ondruch
@voxik commented on this pull request.



>   char *bn = basename(files[i]);
bn[strlen(bn)-strlen(".attr")] = '\0';
fc->atypes[i] = rpmfcAttrNew(bn);
}
+   fc->atypes[nattrs - 1] = rpmfcAttrNew("local_generator");

The `nfiles` could be used here instead, but I am not sure it would make the 
code more readable.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#pullrequestreview-1696861838
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Vít Ondruch
> I have to say, there's beauty to the simplicity of this. Would be even 
> simpler if the new generator was added as the last thing to the array I think.

Done. On top of that, I have added also some test case. The other generator 
abuses `script.attr` AFAICT. Or shell the other test cases be updated to not 
abuse `script.attr` anymore? 樂

I have still left the `local_generator` name around, because it seems there is 
no clear winner yet

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1778907220
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Vít Ondruch
@voxik pushed 1 commit.

61bd40a9df5170da6182e560d172fb16f4e3213b  Add "local_generator"

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/8a532b24e527f48cc45f7f49fc24d6fc4be39d49..61bd40a9df5170da6182e560d172fb16f4e3213b
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Vít Ondruch
@voxik pushed 1 commit.

8a532b24e527f48cc45f7f49fc24d6fc4be39d49  Add "local_generator"

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/33c10c89387b168bceaa93ee2be7c6210a90aa2e..8a532b24e527f48cc45f7f49fc24d6fc4be39d49
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Vít Ondruch
@voxik pushed 1 commit.

33c10c89387b168bceaa93ee2be7c6210a90aa2e  Add "local_generator"

-- 
View it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734/files/4b04cc38167dd637c3c1f68bf6d858453ccf24a1..33c10c89387b168bceaa93ee2be7c6210a90aa2e
You are receiving this because you are subscribed to this thread.

Message ID: 
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Panu Matilainen
Hmm, but just "package" kinda gives the idea that these are the only 
dependencies this package will have, which is not the case. "package_local" 
maybe?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1778645654
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-25 Thread Panu Matilainen
The name should imply this is a per-spec thing, so I don't think "find" is 
good. "local" is much better, but in rpm context I tend to associate that with 
"this host" rather than per package.

"spec" seems accurate, because it is a per-spec thing. But then that makes me 
think of dependencies of the spec itself. Which may even be a thing one day 
(technically, they do have parse-requirements which often is different from 
build-requirements).

How about just "package"? (%__package_requires, %__package_provides etc). That 
seems very much to the point too: these are specific to this package.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1778627591
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-24 Thread Vít Ondruch
> > Subject to name-bikeshedding of course. "local_generator" is not a bad for 
> > what it is, but my head keeps coming up with spec_somethings instead (and 
> > then rejecting) 
> 
> This is the hardest think, right :(

And that is why I have also considered the `find` name, which was already used 
in this context. But of course this might be confusing ...

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1777647182
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-24 Thread Vít Ondruch
> I have to say, there's beauty to the simplicity of this. Would be even 
> simpler if the new generator was added as the last thing to the array I think.

That is an option. Will look at it tomorrow

> Subject to name-bikeshedding of course. "local_generator" is not a bad for 
> what it is, but my head keeps coming up with spec_somethings instead (and 
> then rejecting) 

This is the hardest think, right :(

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1777641462
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


Re: [Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-24 Thread Panu Matilainen
I have to say, there's beauty to the simplicity of this. Would be even simpler 
if the new generator was added as the last thing to the array I think.

Subject to name-bikeshedding of course. "local_generator" is not a bad for what 
it is, but my head keeps coming up with spec_somethings instead (and then 
rejecting) :sweat_smile: 

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734#issuecomment-1777463960
You are receiving this because you are subscribed to this thread.

Message ID: ___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint


[Rpm-maint] [rpm-software-management/rpm] Add "local_generator" (PR #2734)

2023-10-24 Thread Vít Ondruch
This generator can be used by .spec file, which ships their own generators:

~~~
Source1: generator.req
%global __local_generator_requires bash %{SOURCE1}
~~~

Resolves #782
You can view, comment on, or merge this pull request online at:

  https://github.com/rpm-software-management/rpm/pull/2734

-- Commit Summary --

  * Add local_generator

-- File Changes --

M build/rpmfc.c (7)

-- Patch Links --

https://github.com/rpm-software-management/rpm/pull/2734.patch
https://github.com/rpm-software-management/rpm/pull/2734.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/rpm-software-management/rpm/pull/2734
You are receiving this because you are subscribed to this thread.

Message ID: rpm-software-management/rpm/pull/2...@github.com
___
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint