Don't cache github libraries above nginx server.

It's too expensive to fetch the timestamps when the github
server is busy.  See Dick Hollenbeck's comments at the top of
github_plugin.cpp for more info.

Also adds some safety to the other caching algorithms after
seeing github_plugin's wild-west usage of the kicad_plugin.

Fixes: lp:1753143
* https://bugs.launchpad.net/kicad/+bug/1753143
This commit is contained in:
Jeff Young 2018-03-04 01:13:09 +00:00
parent 43147a3a7a
commit 284c346828
13 changed files with 44 additions and 18 deletions

View File

@ -241,7 +241,7 @@ long long FP_LIB_TABLE::GenerateTimestamp( const wxString* aNickname )
{ {
const FP_LIB_TABLE_ROW* row = FindRow( *aNickname ); const FP_LIB_TABLE_ROW* row = FindRow( *aNickname );
wxASSERT( (PLUGIN*) row->plugin ); wxASSERT( (PLUGIN*) row->plugin );
return row->plugin->GetLibraryTimestamp(); return row->plugin->GetLibraryTimestamp( row->GetFullURI( true ) );
} }
long long hash = 0; long long hash = 0;
@ -250,7 +250,7 @@ long long FP_LIB_TABLE::GenerateTimestamp( const wxString* aNickname )
{ {
const FP_LIB_TABLE_ROW* row = FindRow( nickname ); const FP_LIB_TABLE_ROW* row = FindRow( nickname );
wxASSERT( (PLUGIN*) row->plugin ); wxASSERT( (PLUGIN*) row->plugin );
hash += row->plugin->GetLibraryTimestamp(); hash += row->plugin->GetLibraryTimestamp( row->GetFullURI( true ) );
} }
return hash; return hash;

View File

@ -98,9 +98,9 @@ public:
MODULE* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName, MODULE* FootprintLoad( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL ) override; const PROPERTIES* aProperties = NULL ) override;
long long GetLibraryTimestamp() const override long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override
{ {
return getModificationTime( m_lib_path ).GetValue().GetValue(); return getModificationTime( aLibraryPath ).GetValue().GetValue();
} }
bool IsFootprintLibWritable( const wxString& aLibraryPath ) override bool IsFootprintLibWritable( const wxString& aLibraryPath ) override

View File

@ -118,7 +118,8 @@ void FOOTPRINT_LIST_IMPL::loader_job()
bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* aNickname, bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxString* aNickname,
WX_PROGRESS_REPORTER* aProgressReporter ) WX_PROGRESS_REPORTER* aProgressReporter )
{ {
if( aTable->GenerateTimestamp( aNickname ) == m_list_timestamp ) long long libraryTimestamp = aTable->GenerateTimestamp( aNickname );
if( m_list_timestamp == libraryTimestamp )
return true; return true;
m_progress_reporter = aProgressReporter; m_progress_reporter = aProgressReporter;
@ -166,7 +167,7 @@ bool FOOTPRINT_LIST_IMPL::ReadFootprintFiles( FP_LIB_TABLE* aTable, const wxStri
( _( "Loading incomplete; cancelled by user." ), nullptr, nullptr, 0 ) ); ( _( "Loading incomplete; cancelled by user." ), nullptr, nullptr, 0 ) );
} }
m_list_timestamp = aTable->GenerateTimestamp( aNickname ); m_list_timestamp = libraryTimestamp;
m_progress_reporter = nullptr; m_progress_reporter = nullptr;
return m_errors.empty(); return m_errors.empty();

View File

@ -464,6 +464,29 @@ void GITHUB_PLUGIN::cacheLib( const wxString& aLibraryPath, const PROPERTIES* aP
} }
long long GITHUB_PLUGIN::GetLibraryTimestamp( const wxString& aLibraryPath ) const
{
// This plugin currently relies on the nginx server for caching (see comments
// at top of file).
// Since only the nginx server holds the timestamp information, we must defeat
// all caching above the nginx server.
return wxDateTime::Now().GetValue().GetValue();
#if 0
// If we have no cache, return a number which won't match any stored timestamps
if( !m_gh_cache || m_lib_path != aLibraryPath )
return wxDateTime::Now().GetValue().GetValue();
long long hash = m_gh_cache->GetTimestamp();
if( m_pretty_dir.size() )
hash += PCB_IO::GetLibraryTimestamp( m_pretty_dir );
return hash;
#endif
}
bool GITHUB_PLUGIN::repoURL_zipURL( const wxString& aRepoURL, std::string* aZipURL ) bool GITHUB_PLUGIN::repoURL_zipURL( const wxString& aRepoURL, std::string* aZipURL )
{ {
// e.g. "https://github.com/liftoff-sr/pretty_footprints" // e.g. "https://github.com/liftoff-sr/pretty_footprints"

View File

@ -184,6 +184,8 @@ public:
bool IsFootprintLibWritable( const wxString& aLibraryPath ) override; bool IsFootprintLibWritable( const wxString& aLibraryPath ) override;
long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override;
void FootprintLibOptions( PROPERTIES* aListToAppendTo ) const override; void FootprintLibOptions( PROPERTIES* aListToAppendTo ) const override;
// Since I derive from PCB_IO, I have to implement this, else I'd inherit his, which is bad since // Since I derive from PCB_IO, I have to implement this, else I'd inherit his, which is bad since

View File

@ -1120,10 +1120,10 @@ bool GPCB_PLUGIN::FootprintLibDelete( const wxString& aLibraryPath, const PROPER
} }
long long GPCB_PLUGIN::GetLibraryTimestamp() const long long GPCB_PLUGIN::GetLibraryTimestamp( const wxString& aLibraryPath ) const
{ {
// If we have no cache, return a number which won't match any stored timestamps // If we have no cache, return a number which won't match any stored timestamps
if( !m_cache ) if( !m_cache || !m_cache->IsPath( aLibraryPath ) )
return wxDateTime::Now().GetValue().GetValue(); return wxDateTime::Now().GetValue().GetValue();
return m_cache->GetTimestamp(); return m_cache->GetTimestamp();

View File

@ -77,7 +77,7 @@ public:
bool FootprintLibDelete( const wxString& aLibraryPath, bool FootprintLibDelete( const wxString& aLibraryPath,
const PROPERTIES* aProperties = NULL ) override; const PROPERTIES* aProperties = NULL ) override;
long long GetLibraryTimestamp() const override; long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override;
bool IsFootprintLibWritable( const wxString& aLibraryPath ) override; bool IsFootprintLibWritable( const wxString& aLibraryPath ) override;

View File

@ -358,7 +358,7 @@ public:
* directory). * directory).
* Timestamps should not be considered ordered; they either match or they don't. * Timestamps should not be considered ordered; they either match or they don't.
*/ */
virtual long long GetLibraryTimestamp() const = 0; virtual long long GetLibraryTimestamp( const wxString& aLibraryPath ) const = 0;
/** /**
* Function PrefetchLib * Function PrefetchLib

View File

@ -1947,7 +1947,7 @@ void PCB_IO::init( const PROPERTIES* aProperties )
void PCB_IO::validateCache( const wxString& aLibraryPath, bool checkModified ) void PCB_IO::validateCache( const wxString& aLibraryPath, bool checkModified )
{ {
if( !m_cache || ( checkModified && m_cache->IsModified() ) ) if( !m_cache || !m_cache->IsPath( aLibraryPath ) || ( checkModified && m_cache->IsModified() ) )
{ {
// a spectacular episode in memory management: // a spectacular episode in memory management:
delete m_cache; delete m_cache;
@ -2130,10 +2130,10 @@ void PCB_IO::FootprintDelete( const wxString& aLibraryPath, const wxString& aFoo
long long PCB_IO::GetLibraryTimestamp() const long long PCB_IO::GetLibraryTimestamp( const wxString& aLibraryPath ) const
{ {
// If we have no cache, return a number which won't match any stored timestamps // If we have no cache, return a number which won't match any stored timestamps
if( !m_cache ) if( !m_cache || !m_cache->IsPath( aLibraryPath ) )
return wxDateTime::Now().GetValue().GetValue(); return wxDateTime::Now().GetValue().GetValue();
return m_cache->GetTimestamp(); return m_cache->GetTimestamp();

View File

@ -133,7 +133,7 @@ public:
void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName, void FootprintDelete( const wxString& aLibraryPath, const wxString& aFootprintName,
const PROPERTIES* aProperties = NULL ) override; const PROPERTIES* aProperties = NULL ) override;
long long GetLibraryTimestamp() const override; long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override;
void FootprintLibCreate( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL) override; void FootprintLibCreate( const wxString& aLibraryPath, const PROPERTIES* aProperties = NULL) override;

View File

@ -3383,10 +3383,10 @@ void LP_CACHE::LoadModules( LINE_READER* aReader )
} }
long long LEGACY_PLUGIN::GetLibraryTimestamp() const long long LEGACY_PLUGIN::GetLibraryTimestamp( const wxString& aLibraryPath ) const
{ {
// If we have no cache, return a number which won't match any stored timestamps // If we have no cache, return a number which won't match any stored timestamps
if( !m_cache ) if( !m_cache || m_cache->m_lib_path != aLibraryPath )
return wxDateTime::Now().GetValue().GetValue(); return wxDateTime::Now().GetValue().GetValue();
return m_cache->GetLibModificationTime().GetValue().GetValue(); return m_cache->GetLibModificationTime().GetValue().GetValue();

View File

@ -93,7 +93,7 @@ public:
bool FootprintLibDelete( const wxString& aLibraryPath, bool FootprintLibDelete( const wxString& aLibraryPath,
const PROPERTIES* aProperties = NULL ) override; const PROPERTIES* aProperties = NULL ) override;
long long GetLibraryTimestamp() const override; long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override;
bool IsFootprintLibWritable( const wxString& aLibraryPath ) override; bool IsFootprintLibWritable( const wxString& aLibraryPath ) override;

View File

@ -47,7 +47,7 @@ public:
const wxString GetFileExtension() const override; const wxString GetFileExtension() const override;
long long GetLibraryTimestamp() const override long long GetLibraryTimestamp( const wxString& aLibraryPath ) const override
{ {
// No support for libraries.... // No support for libraries....
return 0; return 0;