HPCC-9547 packagemap-validate warning when library redefines query files

In Packagemaps if a query and a library redefine the same file
ecl-packagemap-validate should give a warning.

Signed-off-by: Anthony Fishbeck <anthony.fishbeck@lexisnexis.com>

Conflicts:
	common/workunit/referencedfilelist.cpp
	common/workunit/referencedfilelist.hpp

common/workunit/package.h

@@ -34,10 +34,13 @@ interface IHpccPackage : extends IInterface
 {
     virtual ISimpleSuperFileEnquiry *resolveSuperFile(const char *superFileName) const = 0;
     virtual bool hasSuperFile(const char *superFileName) const = 0;
+    virtual const char *locateSuperFile(const char *superFileName) const = 0;
+
     virtual const char *queryEnv(const char *varname) const = 0;
     virtual bool getEnableFieldTranslation() const = 0;
     virtual const IPropertyTree *queryTree() const = 0;
     virtual hash64_t queryHash() const = 0;
+    virtual const char *queryId() const = 0;
 };
 
 interface IHpccPackageMap : extends IInterface

common/workunit/pkgimpl.hpp

@@ -96,6 +96,11 @@ protected:
         return (node) ? node->hasProp(xpath) : false;
     }
 
+    virtual const char *queryId() const
+    {
+        return (node) ? node->queryProp("@id") : NULL;
+    }
+
     virtual bool getSysFieldTranslationEnabled() const {return false;}
     virtual bool getEnableFieldTranslation() const
     {
@@ -234,20 +239,27 @@ public:
         return NULL;
     }
 
-    bool hasSuperFile(const char *superFileName) const
+    const char *locateSuperFile(const char *superFileName) const
     {
         if (!superFileName || !*superFileName || !TYPE::node)
-            return false;
+            return NULL;
 
         StringBuffer xpath;
         if (TYPE::hasProp(TYPE::makeSuperFileXPath(xpath, superFileName)))
-            return true;
+            return TYPE::queryId();
         ForEachItemIn(idx, bases)
         {
             if (bases.item(idx).hasProp(xpath))
-                return true;
+                return bases.item(idx).queryId();
         }
 
+        return NULL;
+    }
+
+    bool hasSuperFile(const char *superFileName) const
+    {
+        if (locateSuperFile(superFileName))
+            return true;
         return false;
     }
 
@@ -452,13 +464,49 @@ public:
             {
                 Owned<IReferencedFileList> filelist = createReferencedFileList(NULL, NULL);
                 Owned<IConstWorkUnit> cw = wufactory->openWorkUnit(queries->query().queryProp("@wuid"), false);
+
+                StringArray libnames, unresolvedLibs;
+                gatherLibraryNames(libnames, unresolvedLibs, *wufactory, *cw, qs);
+
+                PointerIArrayOf<IHpccPackage> libraries;
+                ForEachItemIn(libitem, libnames)
+                {
+                    const char *libname = libnames.item(libitem);
+                    const IHpccPackage *libpkg = matchPackage(libname);
+                    if (libpkg)
+                        libraries.append(LINK(const_cast<IHpccPackage*>(libpkg)));
+                    else
+                        unmatchedQueries.append(libname);
+                }
+
                 filelist->addFilesFromQuery(cw, this, queryid);
                 Owned<IReferencedFileIterator> refFiles = filelist->getFiles();
                 ForEach(*refFiles)
                 {
                     IReferencedFile &rf = refFiles->query();
-                    if (rf.getFlags() & RefFileInPackage)
+                    unsigned flags = rf.getFlags();
+                    if (flags & RefFileInPackage)
+                    {
+                        if (flags & RefFileSuper)
+                        {
+                            const char *pkgid = rf.queryPackageId();
+                            if (!pkgid || !*pkgid)
+                                continue;
+                            ForEachItemIn(libPkgItem, libraries)
+                            {
+                                IHpccPackage *libpkg = libraries.item(libPkgItem);
+                                const char *libpkgid = libpkg->locateSuperFile(rf.getLogicalName());
+                                if (libpkgid && !strieq(libpkgid, pkgid))
+                                {
+                                    VStringBuffer msg("For query %s SuperFile %s defined in package %s redefined for library %s in package %s", 
+                                        queryid, rf.getLogicalName(), pkgid, libpkg->queryId(), libpkgid);
+                                    warn.append(msg.str());
+                                }
+                            }
+                        }
+
                         continue;
+                    }
                     VStringBuffer fullname("%s/%s", queryid, rf.getLogicalName());
                     unmatchedFiles.append(fullname);
                 }

common/workunit/referencedfilelist.cpp

@@ -71,7 +71,7 @@ class ReferencedFile : public CInterface, implements IReferencedFile
 {
 public:
     IMPLEMENT_IINTERFACE;
-    ReferencedFile(const char *lfn, const char *sourceIP, const char *srcCluster, bool isSubFile=false, unsigned _flags=0) : flags(_flags)
+    ReferencedFile(const char *lfn, const char *sourceIP, const char *srcCluster, bool isSubFile, unsigned _flags, const char *_pkgid) : flags(_flags), pkgid(_pkgid)
     {
         logicalName.set(skipForeign(lfn, &daliip)).toLowerCase();
         if (daliip.length())
@@ -108,9 +108,11 @@ public:
     }
     virtual void cloneInfo(IDFUhelper *helper, IUserDescriptor *user, INode *remote, const char *dstCluster, const char *srcCluster, bool overwrite=false, bool cloneForeign=false);
     void cloneSuperInfo(ReferencedFileList *list, IUserDescriptor *user, INode *remote, bool overwrite);
+    virtual const char *queryPackageId() const {return pkgid.get();}
 
 public:
     StringBuffer logicalName;
+    StringAttr pkgid;
     StringBuffer daliip;
     StringAttr fileSrcCluster;
     unsigned flags;
@@ -129,7 +131,7 @@ public:
         }
     }
 
-    void ensureFile(const char *ln, unsigned flags, const char *daliip=NULL, const char *srcCluster=NULL);
+    void ensureFile(const char *ln, unsigned flags, const char *pkgid, const char *daliip=NULL, const char *srcCluster=NULL);
 
     virtual void addFile(const char *ln, const char *daliip=NULL, const char *srcCluster=NULL);
     virtual void addFiles(StringArray &files);
@@ -400,9 +402,9 @@ public:
     Owned<HashIterator> iter;
 };
 
-void ReferencedFileList::ensureFile(const char *ln, unsigned flags, const char *daliip, const char *srcCluster)
+void ReferencedFileList::ensureFile(const char *ln, unsigned flags, const char *pkgid, const char *daliip, const char *srcCluster)
 {
-    Owned<ReferencedFile> file = new ReferencedFile(ln, daliip, srcCluster, false, flags);
+    Owned<ReferencedFile> file = new ReferencedFile(ln, daliip, srcCluster, false, flags, pkgid);
     if (!file->logicalName.length())
         return;
     ReferencedFile *existing = map.getValue(file->getLogicalName());
@@ -417,7 +419,7 @@ void ReferencedFileList::ensureFile(const char *ln, unsigned flags, const char *
 
 void ReferencedFileList::addFile(const char *ln, const char *daliip, const char *srcCluster)
 {
-    ensureFile(ln, 0, daliip, srcCluster);
+    ensureFile(ln, 0, NULL, daliip, srcCluster);
 }
 
 void ReferencedFileList::addFiles(StringArray &files)
@@ -487,22 +489,28 @@ void ReferencedFileList::addFilesFromQuery(IConstWorkUnit *cw, const IHpccPackag
                 node.getPropBool("att[@name='_isTransformSpill']/@value"))
                 continue;
             unsigned flags = 0;
-            if (pkg && pkg->hasSuperFile(logicalName))
+            if (pkg)
             {
-                flags |= (RefFileSuper | RefFileInPackage);
-                Owned<ISimpleSuperFileEnquiry> ssfe = pkg->resolveSuperFile(logicalName);
-                if (ssfe && ssfe->numSubFiles()>0)
+                const char *pkgid = pkg->locateSuperFile(logicalName);
+                if (pkgid)
                 {
-                    unsigned count = ssfe->numSubFiles();
-                    while (count--)
+                    flags |= (RefFileSuper | RefFileInPackage);
+                    Owned<ISimpleSuperFileEnquiry> ssfe = pkg->resolveSuperFile(logicalName);
+                    if (ssfe && ssfe->numSubFiles()>0)
                     {
-                        StringBuffer subfile;
-                        ssfe->getSubFileName(count, subfile);
-                        ensureFile(subfile, RefSubFile | RefFileInPackage);
+                        unsigned count = ssfe->numSubFiles();
+                        while (count--)
+                        {
+                            StringBuffer subfile;
+                            ssfe->getSubFileName(count, subfile);
+                            ensureFile(subfile, RefSubFile | RefFileInPackage, NULL);
+                        }
                     }
                 }
+                ensureFile(logicalName, flags, pkgid);
             }
-            ensureFile(logicalName, flags);
+            else
+                ensureFile(logicalName, flags, NULL);
         }
     }
 }
@@ -525,7 +533,7 @@ void ReferencedFileList::resolveSubFiles(StringArray &subfiles, bool checkLocalF
     StringArray childSubFiles;
     ForEachItemIn(i, subfiles)
     {
-        Owned<ReferencedFile> file = new ReferencedFile(subfiles.item(i), NULL, NULL, true);
+        Owned<ReferencedFile> file = new ReferencedFile(subfiles.item(i), NULL, NULL, true, 0, NULL);
         if (file->logicalName.length() && !map.getValue(file->getLogicalName()))
         {
             file->resolve(process.get(), srcCluster, user, remote, checkLocalFirst, &childSubFiles, resolveForeign);

common/workunit/referencedfilelist.hpp

@@ -42,6 +42,7 @@ interface IReferencedFile : extends IInterface
     virtual const char *getLogicalName() const =0;
     virtual unsigned getFlags() const =0;
     virtual const SocketEndpoint &getForeignIP(SocketEndpoint &ep) const =0;
+    virtual const char *queryPackageId() const =0;
 };
 
 interface IReferencedFileIterator : extends IIteratorOf<IReferencedFile> { };

common/workunit/workunit.cpp

@@ -9773,6 +9773,34 @@ const char *queryIdFromQuerySetWuid(const char *querySetName, const char *wuid,
     return id.str();
 }
 
+extern WORKUNIT_API void gatherLibraryNames(StringArray &names, StringArray &unresolved, IWorkUnitFactory &workunitFactory, IConstWorkUnit &cw, IPropertyTree *queryset)
+{
+    IConstWULibraryIterator &wulibraries = cw.getLibraries();
+    ForEach(wulibraries)
+    {
+        SCMStringBuffer libname;
+        IConstWULibrary &wulibrary = wulibraries.query();
+        wulibrary.getName(libname);
+        if (names.contains(libname.str()) || unresolved.contains(libname.str()))
+            continue;
+
+        Owned<IPropertyTree> query = resolveQueryAlias(queryset, libname.str());
+        if (query && query->getPropBool("@isLibrary"))
+        {
+            const char *wuid = query->queryProp("@wuid");
+            Owned<IConstWorkUnit> libcw = workunitFactory.openWorkUnit(wuid, false);
+            if (libcw)
+            {
+                names.appendUniq(libname.str());
+                gatherLibraryNames(names, unresolved, workunitFactory, *libcw, queryset);
+                continue;
+            }
+        }
+
+        unresolved.appendUniq(libname.str());
+    }
+}
+
 bool looksLikeAWuid(const char * wuid)
 {
     if (!wuid)

common/workunit/workunit.hpp

@@ -1267,6 +1267,7 @@ extern WORKUNIT_API void deleteQuerySetQuery(const char *querySetName, const cha
 extern WORKUNIT_API const char *queryIdFromQuerySetWuid(const char *querySetName, const char *wuid, IStringVal &id);
 extern WORKUNIT_API void removeQuerySetAliasesFromNamedQuery(const char *querySetName, const char * id);
 extern WORKUNIT_API void setQueryCommentForNamedQuery(const char *querySetName, const char *id, const char *comment);
+extern WORKUNIT_API void gatherLibraryNames(StringArray &names, StringArray &unresolved, IWorkUnitFactory &workunitFactory, IConstWorkUnit &cw, IPropertyTree *queryset);
 
 extern WORKUNIT_API void associateLocalFile(IWUQuery * query, WUFileType type, const char * name, const char * description, unsigned crc);
 

roxie/ccd/ccdstate.cpp

@@ -453,6 +453,10 @@ public:
     {
         return CPackageNode::queryHash();
     }
+    virtual const char *queryId() const
+    {
+        return CPackageNode::queryId();
+    }
 };
 
 typedef CResolvedPackage<CRoxiePackageNode> CRoxiePackage;