Skip to content

Commit

Permalink
Fix loss of attributes for OGR provider (followup 72c9830)
Browse files Browse the repository at this point in the history
  • Loading branch information
nyalldawson committed Jun 29, 2015
1 parent 1b6395e commit cc13c57
Showing 1 changed file with 9 additions and 7 deletions.
16 changes: 9 additions & 7 deletions src/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,7 @@ bool QgsOgrProvider::changeAttributeValues( const QgsChangedAttributesMap &attr_
{
pushError( tr( "OGR error syncing to disk: %1" ).arg( CPLGetLastErrorMsg() ) );
}
QgsOgrConnPool::instance()->invalidateConnections( filePath() );
return true;
}

Expand Down Expand Up @@ -1348,6 +1349,7 @@ bool QgsOgrProvider::changeGeometryValues( QgsGeometryMap & geometry_map )

OGR_F_Destroy( theOGRFeature );
}
QgsOgrConnPool::instance()->invalidateConnections( filePath() );
return syncToDisc();
}

Expand Down Expand Up @@ -1607,17 +1609,17 @@ static QString createFileFilter_( QString const &longName, QString const &glob )

QString createFilters( QString type )
{
/**Database drivers available*/
/** Database drivers available*/
static QString myDatabaseDrivers;
/**Protocol drivers available*/
/** Protocol drivers available*/
static QString myProtocolDrivers;
/**File filters*/
/** File filters*/
static QString myFileFilters;
/**Directory drivers*/
/** Directory drivers*/
static QString myDirectoryDrivers;
/**Extensions*/
/** Extensions*/
static QStringList myExtensions;
/**Wildcards*/
/** Wildcards*/
static QStringList myWildcards;

// if we've already built the supported vector string, just return what
Expand Down Expand Up @@ -2014,7 +2016,7 @@ QGISEXTERN bool isProvider()
return true;
}

/**Creates an empty data source
/** Creates an empty data source
@param uri location to store the file(s)
@param format data format (e.g. "ESRI Shapefile"
@param vectortype point/line/polygon or multitypes
Expand Down

7 comments on commit cc13c57

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n Not sure how this escaped our testing, but this is a nasty one. Any chance of stopping 2.10 and including this in a 2.10.1 release? There's more discussion over at 72c9830

@gioman
Copy link
Contributor

@gioman gioman commented on cc13c57 Jun 29, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this affects also Linux? I use master on a regularly basis on Linux and I haven't seen this bug in the past days. Just noticed today on Windows/2.10.

@jef-n
Copy link
Member

@jef-n jef-n commented on cc13c57 Jun 30, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson where's the backport?

Of course this must popup right after the release, again before the announcement and all packages being built. Although it's were just the LTR builds that were still running - but it's wasn't final-2_8_2 state anyway, but latest on the branch - ie. with your load of 2.10 backports and I just committed mine...
So there should probably be a 2.8.3 soon too.

Should we wait some more for feedback on 2.10? People probably won't try qgis-ltr-dev / debian-nightly-ltr / ubuntugis-nightly-ltr either. :(

@jef-n
Copy link
Member

@jef-n jef-n commented on cc13c57 Jun 30, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there even was a ticket for it.

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n I don't think the original issue in that ticket is directly related - it refers to an issue in 2.8, and as far as I'm aware the offending commit wasn't backported to 2.8. But dash WAS reporting failures to us. It's just unfortunate that they got drowned in the noise of other failing OSX tests. I'm working on resolving that now...

@jef-n
Copy link
Member

@jef-n jef-n commented on cc13c57 Jun 30, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson if that was actually intended. 2.8.2 was the default for new tickets. I removed the default and now it appears with Please select...

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n I've backported in 3738c91

Please sign in to comment.