- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 1.8k
 
gnome: Look for new gobject-introspection tools names #15101
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
          
     Draft
      
      
            xclaesse
  wants to merge
  1
  commit into
  mesonbuild:master
  
    
      
        
          
  
    
      Choose a base branch
      
     
    
      
        
      
      
        
          
          
        
        
          
            
              
              
              
  
           
        
        
          
            
              
              
           
        
       
     
  
        
          
            
          
            
          
        
       
    
      
from
xclaesse:gnome-glib-gi
  
      
      
   
  
    
  
  
  
 
  
      
    base: master
Could not load branches
            
              
  
    Branch not found: {{ refName }}
  
            
                
      Loading
              
            Could not load tags
            
            
              Nothing to show
            
              
  
            
                
      Loading
              
            Are you sure you want to change the base?
            Some commits from the old base branch may be removed from the timeline,
            and old review comments may become outdated.
          
          
  
     Draft
                    Changes from all commits
      Commits
    
    
  File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
      
      Oops, something went wrong.
        
    
  
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the way
_get_gir_depworks, if you are pulling glib as a subproject, it'll callgenerate_girand_get_gir_depwill hit this branch and setself.gir_dep. Any further calls to it will always use this shimmed version even tho the proper one would be available. This will cause issues otherwise if the specific library you are building introspection for is only linked against a subset of {glib,gobject,gio} (error while loading shared libraries: libgio-2.0.so.0: cannot open shared object file: No such file or directory).The use of girepository-2.0 also seems a bit questionable, as AFAIK it actually isn't needed to generate introspection (Even tho it provides
gi_repository_dump, the scanner links its owngdump.c).Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true, I'm undecided on that yet. Ideally I think generate_gir() should stop having implicit dependency on glib girs, but that's not backward compatible: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/9824.
Since glib now uses generate_gir() itself to generate its girs, if we want to keep backward compatiliby we should special case glib/gobject gir generation to store their generated target and add that to any future call to generate_gir().
It is only needed to get the "girdir" pkgconfig variable. I don't think glib exposes that value anywhere else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is fundamentally a dependency on gio-2.0 & gmodule-2.0 cause giscanner does itself not sure why gio-2.0, given that
gdump.conly includesglib.h,glib-object.handgmodule.h, bu oh well 🤷♂️. But those aren't necessarily the girs, but the libraries themselves.I was gonna say this isn't necessary anymore, given that for internal dependencies you can just add the girs in question as sources, but I guess it is indeed needed for pkg-config.
I'll have to think about it some more but perhaps we can attach the glib girs as source to the girepo dependency (it'll take some finagling, as generare_gir needs the typelib compiler, which does need girepo)
Sounds horrifying, I'd prefer a solution where
_get_gir_depsimply doesn't cache the temporary shim. The downside is that meson will spam 'Failed to find dependency girepository-2.0' a dozen times while configuring glib.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g-ir-scanner's py module links to gio, but that's fine,
generate_gir()does not need to know that. At least once we uselocal_program. The problem forgenerate_gir()is it used to also implicitly give glib GIRs which can't be done when those GIRs are themself built with built withgenerate_gir().But I'm not sure every GIR needs to include glib/gobject GIR, does it? I think that implicit dependecy was a mistake.
Exactly, that's to find installed GIRs.
I know... but the alternative to not cache is not great either. We can also just assume to break backward compat, but then we need MRs like this one everywhere: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/9824.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generate_gir does need to know that, as its not just the py module that links to gio, but also the dumper program that the scanner compiles during invocation. And that needs to be the host machine gio, unlike the gio for the g-ir-scanner could also be from the build machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, that's right. The way that works is g-ir-scanner uses pkg-config and Meson setup its env to find
glib-2.0-uninstalled.pcfile. So we don't need a direct dependency for that.To generate glib's GIRs it does pass the dependency explicitly, so that's fine: https://gitlab.gnome.org/GNOME/glib/-/blob/main/girepository/introspection/meson.build?ref_type=heads#L89.
The issue is when building 3rd party GIRs (e.g. gstreamer), Meson should add the GIR generated by glib as dependency automatically, or break backward compat and require they add the dependency explicitly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, meson needs to ensure that dependency ordering is still kept and libglib-2.0.so & etc. is built before the scanner is run. Using
girepository-2.0obv. does achieve this.You can't just break backwards compatibility. There are 2.1k occurrences of generate_gir on github alone (now most of them aren't using subprojects, but a few of 2.1k is still a lot). But I really don't see why attaching the GIRs to either
girepository-2.0or a new separategi-base-1.0(for cases where the g-i subproject is evaluated first) is that big of a deal, esp. since I don't think the expectation of having the core GLib libraries directly available is unreasonable. I'm more concerned about the other girs that are part of g-i (cairo, fontconfig, xlib, etc.) cause they have way less of an expectation of being directly available and are probably also used a lot less. Nontheless, its probably not a good idea to break backwards compatibility.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That cannot be done until after glib generated its girs, which is using gnome.generate_gir(). That means we should either not cache the result of checking for
girepository-2.0orgi-base-1.0dependency. Or we should special-case glib GIRs ingnome.generate_gir()to add them as dependency for future calls ofgnome.generate_gir(). You said "Sounds horrifying", but I think we have no choice TBH.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I very much dislike the idea of somehow checking if we are currently building a glib gir and add them to a cache, as that'll sooner or later cause issues when someone makes GIR additions/changes in glib. While the not caching the result of
_get_gir_depuntil we findgirepository-2.0orgi-base-1.0has some issues (red dependency not found spam (which scares some people) while building glib and fun things happening if--force-fallback-for girepository-2.0/gi-base-1.0is set) I'd much prefer it.